-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feat: claims verification #11
Conversation
Coverage reportClick to see where and how coverage changed
This report was generated by python-coverage-comment-action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verification features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed the code and tested the feature. Looks good to me
elif claim_value.lower() != "false": | ||
claims.append(f"{claim}:{claim_value}") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought: if we're doing this for "false"
, maybe we should also do it for 0
?
This doesn't need to be changed for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(I don't know if this is 100% right because the true/false
claims are new for us -- cc @johnatstate)
Here's my thinking on the integer cases:
- Store the name of the claim if
value == 1
(success) - Store the name and the value if
value >= 10
(error) - We don't store anything if
value == 0
(fail)
And so this is analogous for the non-integer cases:
- Store the name of the claim if
value == true
(success) - Store the name and the value if
value != false
(an actual value, like[email protected]
) - We don't store anything if
value == false
(fail)
That last two bullets are the assumption -- I don't know if we would get false
back as a string, but I saw true
so added the corollary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh I see. Thanks for the clarification!
What this PR does
Introduces a new model
web.core.models.UserFlow
to hold details about the flow a user will go through (e.g. Vital records request)UserFlow
contains the scopes and claims necessary for communication with IdGUserFlow
maps its own URL routes viasystem_name
andurlconf_path
UserFlow
instancesImplements claims verification similar to
benefits.oauth
andbenefits.eligibility.verify
Implements session handling to maintain OAuth related config between requests
Testing locally
.env
with details for thedev
IdGbin/build.sh
F5
to launchVital records
option is in the top bar menuVital records
to be directed into that flowproofing-disaster-affected.yml
andproofing-disaster-not-affected.yml
for relevant Sandbox accountsdisaster-affected
Login.gov account, expect the "Verified" pagedisaster-not-affected
Login.gov account, expect the "Unverified" page