Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Validating acr values (specifically idp) in TokenRequestValidator #2096

Closed
ghost opened this issue Feb 22, 2018 · 5 comments
Closed

Validating acr values (specifically idp) in TokenRequestValidator #2096

ghost opened this issue Feb 22, 2018 · 5 comments
Labels

Comments

@ghost
Copy link

ghost commented Feb 22, 2018

On a standard front channel request in AuthorizeRequestValidator there is logic for "check custom acr_values: idp" to ensure that if a client has idp restrictions that the request contains an idp and it is in that restriction list.

All working as expected.

When moving on to implementing the resource owner flow I was expecting the same checks to be made in the TokenRequestValidator however there doesn't appear to be any logic for checking the same acr values.

I have since gone and included that logic in my implementation of IResourceOwnerPasswordValidator however I was wondering if despite respecting acr values and implementing idp restrictions elsewhere, that this was potentially an oversight or was there an intentional reason to not validate that value there.

Cheers,

@brockallen
Copy link
Member

When moving on to implementing the resource owner flow I was expecting the same checks to be made in the TokenRequestValidator however there doesn't appear to be any logic for checking the same acr values.

That's hard to do in resource owner flow -- for example, there's not way in resource owner to accept google credentials. Also, resource owner does not travel the UI workflow, and it's not technically OIDC.

@ghost
Copy link
Author

ghost commented Feb 23, 2018

That's hard to do in resource owner flow -- for example, there's not way in resource owner to accept google credentials.

Ahhhh... I guess that makes sense. Our implementation exclusively respects internal credential stores, so we always have the ability to handle any idp via the resource owner flow... which wouldn't be the case for something like Google.

In that case can you see any obvious problems with that checking logic existing in the IResourceOwnerPasswordValidator implementation?

@brockallen
Copy link
Member

I guess I don't know how you're using the idp acr_value then, as it's designed to assist when using external IdP.

@ghost
Copy link
Author

ghost commented Feb 26, 2018

I guess I don't know how you're using the idp acr_value then, as it's designed to assist when using external IdP.

Yeah that's fair. I guess our scenario is a little out of the ordinary (treating internal credential stores as if they were external but still allowing RO flow). Thanks for your comments.

@ghost ghost closed this as completed Feb 26, 2018
@github-actions
Copy link

github-actions bot commented Jun 1, 2021

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 1, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

1 participant