Conversation
Adds ial and all to OIDC JWT response
Clean up debugging stuffs
additional cleanup
Jeremy1026
changed the title
…esponse Include IAL/AAL string value instead of integer value in Userinfo
Jeremy1026
force-pushed
the
jcurcio/lg-8704-add-ial-and-aal-to-userinfo-response
branch
from
4301a51 to
c6913ad
Compare
db/primary_migrate/20230206172633_add_default_value_to_identity_aal.rb
Outdated
Show resolved
Hide resolved
orenyk
changed the title
Jeremy1026
force-pushed
the
jcurcio/lg-8704-add-ial-and-aal-to-userinfo-response
branch
from
828d7d3 to
0a51dbf
Compare
db/primary_migrate/20230206172633_add_default_value_to_identity_aal.rb
Outdated
Show resolved
Hide resolved
orenyk
left a comment
orenyk
left a comment
There was a problem hiding this comment.
Let's drop the migration, I think this should be good otherwise!
db/primary_migrate/20230206172633_add_default_value_to_identity_aal.rb
Outdated
Show resolved
Hide resolved
Lint is probably going to hate the line length
orenyk
left a comment
orenyk
left a comment
There was a problem hiding this comment.
One quick comment re: where the default behavior lives. I also just noticed that there are no unit tests for this functionality in the presenter (and the form if you move the default behavior there) - if you could add those and re-add the feature spec checking for the attributes in the userinfo response this should be good to go!
orenyk
left a comment
orenyk
left a comment
There was a problem hiding this comment.
Two minor comments on the form spec but otherwise good to go from my perspective!
| end | ||
|
|
||
| describe '#aal' do | ||
| context 'when AAL1 passed' do |
There was a problem hiding this comment.
suggestion: I don't think we need to test for this since we don't support it
There was a problem hiding this comment.
Actually, this should probably be replaced with a test for the default AAL specification instead of AAL1
There was a problem hiding this comment.
Is there any harm in keeping checking 1? A couple extra cycles, and an extra second per test run?
There was a problem hiding this comment.
nah not really, other than the setup where you modify the allowed aal values feeling a bit weird to me. Either way is fine by me!
Jeremy1026
deleted the
jcurcio/lg-8704-add-ial-and-aal-to-userinfo-response
branch
5 participants
🎫 Ticket
LG-8704
🛠 Summary of changes
Change user info in the OIDC token response to include the URLs for the IAL and AAL instead of the integer value.