Skip to content
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

add session_required_policy to AuthorizationParamaterInfo #658

Merged
merged 2 commits into from
Dec 7, 2022

Conversation

aaschaer
Copy link
Contributor

@aaschaer aaschaer commented Dec 5, 2022

No description provided.

kurtmckee
kurtmckee previously approved these changes Dec 5, 2022
@kurtmckee kurtmckee dismissed their stale review December 5, 2022 17:53

Tests failing

@aaschaer aaschaer force-pushed the aaron/auth_policies branch 2 times, most recently from 9dbe4cf to b6e8809 Compare December 5, 2022 20:10
Copy link
Member

@sirosen sirosen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I'm good with this being our approach here.

If we find that some authorization_parameters fields are used exclusively in many (or all?) cases, we may want to pull that up into a (duplicate) ErrorInfo object, like err.info.session_required_policies. I'll need to learn more about the design here to be able to comment on what makes sense.

For now, this exposes the data which we know about, so I'm happy with it. 👍

@sirosen sirosen merged commit 9fbd80d into main Dec 7, 2022
@sirosen sirosen deleted the aaron/auth_policies branch December 7, 2022 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants