-
Notifications
You must be signed in to change notification settings - Fork 32
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
Integration of OIDC Flow via authorization_code grant #477
Comments
Thank you so much for such a detailed description, @denniskniep! I think we should put OIDC Flows in Authorino to a vote. Hoping not to influence the process, I'll abstain in the vote and just leave here my personal take on the proposed changes, separated by topics: OIDC Flows Personally, I believe it should be part of Authorino's philosophy to be as transparent as possible to the API consumer, working rather on behalf of the server instead. Negotiating tokens on behalf of the consumer goes on the opposite direction, shifting Authorino's role, at least around this particular feature, towards becoming a proper client the consumer delegates power to (in OAuth2 terms). Another concern of mine regards to the Automatic refresh token (listed as future improvement) Authorino storing and managing consumers' refresh tokens raises yet another flag IMO. It means handling state beyond caching, thus adding new requirements. Cookie Response I think this is a valid addition worth extracting to a separate issue. Possibly, it would expand to modifying the response to the consumer beyond cookie setting (other headers and maybe body too?), but definitely introducing clearer distinction between this and modifying the request to the upstream (already supported). I see this change as valuable to multiple other applications as well. One who's feeling adventurous enough could try combining it to anonymous access and OPA, and get full OIDC API changes We're now in the midst of rolling out a series of releases to push the AuthConfig API (as well as Authorino itself) to v1. At this point, additions to the API should target |
Thanks for your feedback @guicassolato. Two comments:
I think it depends on the type of application that Authorino is serving. There are servers which actually do the OIDC authorization_code grant flow and not only consuming the token. In that particular scenario Authorino would act on behalf of the server.
I think a dedicated endpoint is not necessary. Because the callback happens within a redirect and this is always a |
This feature sounds like it would solve some challenges I've run into and it would be great if it could be incorporated. |
OIDC Flow
As-is Situation
Currently authorino is able to authenticate requests based on provided credentials by extracting them from the ext auth check request.
i.e extracting an jwt from the http header and verifying the signature. If successful than the authorino pipeline continues to the next stage.
Feature Request
It would be useful if authorino would be able to handle the flow for issuing IdTokens via OIDC Flow. Utilizing the OIDC standard
Motivation
Envoy has already an OAuth2 Filter which fulfills the basic requirements.
But currently it seems to be that there is not much appetite for making the OAuth2 Filter more feature rich. If someone wants to contribute a feature on their own, he need to catch a signer (envoy maintainer) and need to write that feature as C++ code.
The expectation is, that it will be easier to extend OIDC Flow inside authorino. Furthermore there will be then one single config which is repsonsible for AuthN & AuthZ and not spread accross several components configs.
Initial Scope of the Change
Handle oidc flow and issue IdToken with
authorization_code grant
. ClientId & Secret are sent via Basic Auth. The issued jwt can be set as httponly cookie to downstream, so that subseqeuent requests can skip the full flow.Changes
API changes
Add
OidcFlow
config property toAuthenticationMethodSpec
Add Cookie Response
Providing an easy to use config option for issuing cookies with Custom Responses
It might be useful, if we could set the
Expires
property of the cookie based on theexpiresAt
value from the IdToken. Therefore usingValueOrSelector
as data type.Framework changes
Option to return
AuthResult
fromIdentityPipeline
We need to influence the
AuthResult
from thepipeline.evaluateIdentityConfigs()
to set specific status code and header to downstream client(More precisely we need to respond with a redirect to the IDP, if the user does not provide any authentication information)
Proposal:
Create an wrapped error struct
ErrorDenyWith
with the following properties:
If the returned error by
pipeline.evaluateIdentityConfigs()
is of typeErrorDenyWith
then it is overwriting the user configured
pipeline.AuthConfig.Unauthenticated
response! TheErrorDenyWith
from the highest configuration priority wins.Flow
GetCredentialsFromReq()
. If yes, utilize logic fromidentity.OIDC
AuthResult
with Authentication request which is redirecting to the IDP.CallbackPath
.Authorization JSON
Improvements for Later
Improvements, that we can discuss and address later, because they are not essential for an intial MVP. Therefore they are explicitly out of scope:
headers_to_remove
)Furthermore it might be a useful feature to introduce Sessions into authorino. So that the IdToken never leaves the authorino backend (i.e. inside a cookie).
The text was updated successfully, but these errors were encountered: