This issue is from discussion that happened around issue #54 and PR #55, and changes for consideration after meeting the feature parity milestone.
Discussion was around handling prompt and max_age parameters during an authorize request, particularly the case where max_age=0. Specifically with max_age=0, there exists a bug, also present in identityserver4, where the end user gets stuck in a login loop as max_age=0 is handled on the callback and re-triggers additional login.
It is desirable to not strip away these parameters prior to presenting a login screen, as this enables bespoke messaging about why the login screen is being presented.
Current solution has been to add additional constants to the parameter collection to signal that these parameters have been processed so that they can be skipped on the callback.
Suggested rework for consideration:
To get rid of the constants and add something like RequestType to the ValidatedAuthorizeRequest. The property could be an enum (maybe AuthorizeRequestType) with values like Authorize, PushedAuthorize, CibaAuthorize (in the future), but also AuthorizeCallback. When the value is set to AuthorizeCallback we could skip processing of prompt, max_age=0, but still validate all max_age values greater than 0.
Alternatively, separating into AuthorizeRequestContext objects could enable a larger refactoring of request validation, allowing it to be broken into several smaller classes.
This issue is from discussion that happened around issue #54 and PR #55, and changes for consideration after meeting the feature parity milestone.
Discussion was around handling
promptandmax_ageparameters during an authorize request, particularly the case wheremax_age=0. Specifically withmax_age=0, there exists a bug, also present in identityserver4, where the end user gets stuck in a login loop asmax_age=0is handled on the callback and re-triggers additional login.It is desirable to not strip away these parameters prior to presenting a login screen, as this enables bespoke messaging about why the login screen is being presented.
Current solution has been to add additional constants to the parameter collection to signal that these parameters have been processed so that they can be skipped on the callback.
Suggested rework for consideration:
To get rid of the constants and add something like RequestType to the ValidatedAuthorizeRequest. The property could be an enum (maybe AuthorizeRequestType) with values like Authorize, PushedAuthorize, CibaAuthorize (in the future), but also AuthorizeCallback. When the value is set to AuthorizeCallback we could skip processing of prompt, max_age=0, but still validate all max_age values greater than 0.
Alternatively, separating into AuthorizeRequestContext objects could enable a larger refactoring of request validation, allowing it to be broken into several smaller classes.