Allow OIDC prompt query param

[tdg5]

The OpenID Connect Core spec defines an extension to the authorization code grant type that allows that grant type to take an optional prompt query parameter that allows the client to have more control over how the auth server handles an authorization code request in scenarios where the user is not signed in or has not consented to all the scopes.

I think the handling of the prompt query argument is the responsibility of whomever is using this library, but I wonder if you'd be open to adding some support for the prompt query argument to this library. I tried to handle prompt without getting this library involved, but starlette's QueryParams class is immutable, so I don't have a good means of removing the prompt query argument from the request before passing the request on to this library.

This PR currently reflects the bare minimum needed to make this library play nicely with the prompt query parameter, but I'd be happy to add some validation of the prompt query parameter to AuthorizationCodeGrantType.validate_request if you are open to aioauth including some basic support for the prompt query param.

Here's what the OIDC Core spec has to say about prompt:

prompt
  OPTIONAL. Space delimited, case sensitive list of ASCII string values that
  specifies whether the Authorization Server prompts the End-User for
  reauthentication and consent. The defined values are:
    none
      The Authorization Server MUST NOT display any authentication or consent
      user interface pages. An error is returned if an End-User is not already
      authenticated or the Client does not have pre-configured consent for the
      requested Claims or does not fulfill other conditions for processing the
      request. The error code will typically be login_required,
      interaction_required, or another code defined in Section 3.1.2.6. This
      can be used as a method to check for existing authentication and/or
      consent.
    login
      The Authorization Server SHOULD prompt the End-User for reauthentication.
      If it cannot reauthenticate the End-User, it MUST return an error,
      typically login_required.
    consent
      The Authorization Server SHOULD prompt the End-User for consent before
      returning information to the Client. If it cannot obtain consent, it MUST
      return an error, typically consent_required.
    select_account
      The Authorization Server SHOULD prompt the End-User to select a user
      account. This enables an End-User who has multiple accounts at the
      Authorization Server to select amongst the multiple accounts that they
      might have current sessions for. If it cannot obtain an account selection
      choice made by the End-User, it MUST return an error, typically
      account_selection_required.
  The prompt parameter can be used by the Client to make sure that the End-User
  is still present for the current session or to bring attention to the
  request. If this parameter contains none with any other value, an error is
  returned.

[codecov-commenter]

Codecov Report

Merging #75 (871f762) into master (48896f2) will increase coverage by 0.00%. The diff coverage is 100.00%.

:mega: This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@           Coverage Diff           @@
##           master      #75   +/-   ##
=======================================
  Coverage   98.78%   98.78%           
=======================================
  Files          14       14           
  Lines         658      659    +1     
  Branches       99       99           
=======================================
+ Hits          650      651    +1     
  Misses          4        4           
  Partials        4        4           

Impacted Files	Coverage Δ
aioauth/requests.py	100.00% <100.00%> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

[aliev]

This PR currently reflects the bare minimum needed to make this library play nicely with the prompt query parameter, but I'd be happy to add some validation of the prompt query parameter to AuthorizationCodeGrantType.validate_request if you are open to aioauth including some basic support for the prompt query param.

sure, approved. you can add the implementation within this or next PR.

[tdg5]

I'm still evaluating how I want to extend this. I'm fine to keep this PR open if it's all the same to you.

[tdg5]

I've been thinking this over a bit today and I'm inclined to say that the way that authlib and oauthlib handle the different standards they support is what I'd recommend for this library as well: handle concerns specific to one standard in a standard specific class. A feature flag for OIDC begets a feature flag for RFC6750 and so on. We'd be falling into the trap that this refactoring aims to remedy: Replace conditional with polymorphism. It also would make tests more annoying by increasing the number of standard specific configuration standards that need to be tested.

So, though what I've suggested in this PR is simple, I'd argue that it moves this library a small step in the wrong direction. What I would instead propose is beginning to migrate the OIDC specific behaviors into OIDC specific subclasses. I get the impression that creating a new subclass of Query and a new subclass of BaseRequest would do the trick, however it runs into a problem at the level of aioauth-fastapi which is fixed on using the standard Request, Query, and Post classes. But that's a problem for that repo.

[tdg5]

@aliev, I've updated this PR in such a way as to begin the road toward handling standard specific behaviors in standard specific subclasses for various components. Let me know what you think. If you're happy with what's here, I'd request that you merge this PR and I can use it as a base for migrating some of the id_token work I have open in #76.

I'm still trying to figure out what needs to happen in aioauth-fastapi to make it better able to work with standard specific subclasses, but I don't think that's something that needs to block this PR. EDIT: I think I've resolved how this would work with aioauth-fastapi via https://github.com/aliev/aioauth-fastapi/pull/10

[tdg5]

@aliev , I think this is ready for your review whenever you have the opportunity.

[tdg5]

@aliev, I don't mean to rush you on this, but I want to make sure we're aligned on what's possible here.

I know that you granted me some kind of permissions for this repo, but I'm not able to move this code forward without a review from a code owner.

If you intend to get to this PR and haven't had a chance, that is totally cool, but I just want to make sure you're not expecting me to take care of this on my own.

[aliev]

@aliev, I don't mean to rush you on this, but I want to make sure we're aligned on what's possible here.

I know that you granted me some kind of permissions for this repo, but I'm not able to move this code forward without a review from a code owner.

I have added you as a collaborator so that the pipelines can run automatically without my approval.

If you intend to get to this PR and haven't had a chance, that is totally cool, but I just want to make sure you're not expecting me to take care of this on my own.

Sorry for the delay. Please give me some time, I will double-check the entire changes.

[tdg5]

@aliev , thanks for clarifying! There's really no rush, I just wanted to make sure you didn't expect me to take action on my own.

[aliev]

I approved this PR and will merge it. Overall, everything looks good. The only thing I would suggest is to add a docstring to the prompt field and the Query class in the aioauth/oidc/core/requests.py file, with a reference to the specification.

However, I think this can be done in your next PR, so as not to delay your progress.