Open saschanaz opened 1 year ago
This is implementation detail, the allowed flag is set by client implementation and indicates approval if true or denied of false.
It indeed is implementation detail, but I see no way to:
authorize()
(in a route handler)I feel like the library expects the client to access the authorization endpoint after having approval from the user somehow, is that correct?
@saschanaz it's the second to third step in the Authorization Code Grant workflow: https://datatracker.ietf.org/doc/html/rfc6749.html#section-4.1 see Part B)
Yes I want to do that, but how can implement that using this library? oauth2orize for example allows rendering a page after the parameter verification with a transaction ID (https://github.com/jaredhanson/oauth2orize#implement-authorization-endpoint).
Edit: to be more specific:
(B) The authorization server authenticates the resource owner (via the user-agent) and establishes whether the resource owner grants or denies the client's access request.
For this step, in many cases the server renders a page for user with buttons to grant/deny, or login if not yet. I don't immediately see such step in this library, to render an (implementation detail) page before granting code. AFAICT this library just proceeds to grant, doesn't it?
@saschanaz do you use express? We could create a minimal example project with express + vanilla JavaScript + in memory DB that shows how this could work. We have an examples repo under https://github.com/node-oauth/node-oauth2-server-examples and this would be a perfect example. What do you think?
I use Fastify but can use fastify-express if needed. Yes, an example would be great.
@saschanaz I added you as collaborator to the examples repo and created a branch there incl. draft PR: https://github.com/node-oauth/node-oauth2-server-examples/pull/1
Let's continue discussion here: https://github.com/node-oauth/node-oauth2-server-examples/issues/2
I had/am having similar issues. I can't believe this is so difficult to get going (that is not a criticism of the project maintainers, just a comment on the project itself).
After hours digging into the source code I found I needed to implement:
....authorize({
authenticateHandler: {
handle: function(req, res) {
// get authenticated user or return falsy value, e.g.:
return req.session.user;
}
}
taken from https://github.com/oauthjs/node-oauth2-server/issues/314
A good, and complete, example would be a great first step though.
To the maintainers: Thanks for keeping this project alive and doing what you do! 👍
Is it related to showing the dialog page during the authorization process, though? It's only to extract the user info from the request, right?
Well yeah sort of... I was trying to work out how to get the auth code. You can't get an auth code without having an authorised user (I'm still working through this so if this isn't 100% please correct me). If you don't override that function an exception will be thrown as it expects a token to be present.... ...but there can't be a token as you are trying to access this resource for the first time (as per the related issue in the original oauthjs project).
but there can't be a token as you are trying to access this resource for the first time (as per the related issue in the original oauthjs project).
Exactly, and it should be perfectly fine to not have a token since the client may be a native app and the browser may not have any user info yet when opening oauth/authorize endpoint.
@saschanaz @jfstephe just to clarify: we are all talking about the authorization code grant
workflow here, right?
Yes, and see https://datatracker.ietf.org/doc/html/rfc8252#section-4.1 for example (as IMO it's clearer than the one in https://datatracker.ietf.org/doc/html/rfc6749.html#section-4.1).
(1) Client app opens a browser tab with the authorization request.
(2) Authorization endpoint receives the authorization request, authenticates the user, and obtains authorization. Authenticating the user may involve chaining to other authentication systems.
A client in the step 1 will, for example, open https://host.example/oauth/authorize?...
via a web browser. And this request should be validated and then render something in the browser to show accept/reject buttons. So far I see no way, and oauth-example skips the whole validation and goes straight to show the form, which seems wrong to me.
https://datatracker.ietf.org/doc/html/rfc6749.html#section-4.1.1
The authorization server validates the request to ensure that all required parameters are present and valid. If the request is valid, the authorization server authenticates the resource owner and obtains an authorization decision (by asking the resource owner or by establishing approval via other means).
Validate and then obtain the decision, not the other way around, right?
@saschanaz if you are still interested then please let's continue this in the examples repository. Closing this for now.
I'll describe the issue as I understood it. Let's see if this is what @saschanaz meant.
The flow right now:
app.use("/authorize", ensureLoggedIn(), oauth.authorize());
Need to prompt scope for user's verification before issuing an authorization code. My current options right now are:
app.use("/authorize", ensureLoggedIn(), promptScope(), oauth.authorize());
or
app.use("/authorize", ensureLoggedIn(), oauth.authorize(), promptScope());
Let's talk about each of the options.
We prompt scope before authorizing the code request:
app.use("/authorize", ensureLoggedIn(), promptScope(), oauth.authorize());
In this case, we have user's session -- which is required before we prompt the user a dialogue to allow/reject the scope -- but we don't have the client yet, as it is only validated later, during authorize()
call. Therefore, the user would have to approve the scope without knowing what client they are approving it for. Alternatively, a developer would need to copy-paste the client validation functionality into promptScope
. Not the end of the world, but it causes code duplication.
We have user session, and we validate both the user and the client:
app.use("/authorize", ensureLoggedIn(), oauth.authorize(), promptScope());
In this case, the authorization code is already granted. Prompting the user at this point is possible, but a developer would need to remove the issued code from the data store in case of scope rejection by the user. Again, ok but not perfect.
What we want:
app.use("/authorize", ensureLoggedIn(), oauth.authorize());
^
scope validation prompt here
It should go in the place where both the user session and the client are validated, but the authoriztion code hasn't been issued yet -- and won't be if the user rejected the scope request prompt.
@jankapunkt I would be happy to learn that what I am trying to do is possible with the current server implementation, but seems like this hasn't been thought through. Any guidance is much appreciated.
Hello @sergeyshilin ,
The first option is actually a viable option. On the statement "but we don't have the client yet, as it is only validated later", I think the problem is more with using a single endpoint for prompting and authorize. On my implementations, I usually have separate APIs for these with a dedicated frontend. For instance, consider the following flow:
client_id
in the URL params or even have a default client_id
set in your frontend.get-client
API that will return a very minimal info about the client like scopes it requires, it's display name, etc. authorize
API.As you might have noticed, consent screen is a completely isolated from the core working of the library and is an implementation detail. The authorize API is called only after the user has consented.
Of course, this is just my implementation, but maybe @jankapunkt has a better way of doing this.
Thanks @shrihari-prakash, that makes sense to me! Although it feels a bit like a workaround due to client information being validated twice, it is still a working solution indeed.
Thanks @shrihari-prakash, that makes sense to me! Although it feels a bit like a workaround due to client information being validated twice, it is still a working solution indeed.
I see where you're coming from. I imagine you could also do away without using an API to fetch the client details as well. Except that you send scopes along in the login page and show that list on the consent screen as well. In any case if the scope is not valid, OAuth server takes care of rejecting the request in authorize call subsequently.
The main thing I was trying to convey is that anything that happens before authorize, say login or a consent screen is outside of the library's responsibilities. It's not something I would closely couple with the authorize API.
say login or a consent screen is outside of the library's responsibilities
I disagree with this statement. Approving/rejecting scopes is part of the OAuth2 specification, so it would make sense to have it supported within the library too.
An application can request one or more scopes, this information is then presented to the user in the consent screen, and the access token issued to the application will be limited to the scopes granted.
For example, as @saschanaz mentioned earlier, it is part of oauth2orize -- another node oauth server library -- by design: https://github.com/jaredhanson/oauth2orize#implement-authorization-endpoint
approving and rejecting scopes is partially implemented to the point where the standard demands it. Beyond that we allow users to have fine grained control over scopes using model implementation.
Everything beyond that, especially any involvement of UI layers is up to the developers.
Regarding allow/deny - I think we can talk about a better integration of passing the values on to the model but I personally don't see an implementation that might ease up the process a lot. What do you currently imagine for a helpful behavior?
The main thing I was trying to convey is that anything that happens before authorize, say login or a consent screen is outside of the library's responsibilities. It's not something I would closely couple with the authorize API.
There is no point to show any screen to user before validation. Say, the server allows scope A and B and somehow the app requests A/B/C. My expectation is that the request fails without prompting anything to user, which is currently impossible without duplicating the validation steps. Perhaps the validation part should be split from authorize()?
@saschanaz what do you mean by "duplicating"? You mean because of verifyScope
and validateScope
?
That and maybe all the checks here that should ideally happen before showing any screen:
@jankapunkt duplication of validation comes from validating twice both the scope (the requested scope is in the list of server's accepted scopes), and the client (the client_id
a valid string, and the client itself exists in the DB).
Twice because the first time is for prompting the scope to a user, as per the picture
and second time when calling authorize()
.
Instead, an expected flow would be
authorize()
In my implementation of an OAuth server using node-oauth2-server
, I did it this way:
/oauth/authorize
endpoint is implemented by the frontend, which takes the query parameters and calls an internal API via Ajax, which in turn calls authorize()
to do all the validation before the consent screen is even shown.saveAuthorizationCode
saves the generated authorization code in the database, marks the request as "needing user consent", and returns data needed to render the consent screen.authorize()
would have returned to in the HTTP Location
header.window.location.replace
.Rube Goldberg would be proud. Unfortunately, this is the most unopinionated maintained implementation of OAuth2 in Node that I could find, and the most well-documented one. oauth2orize
expects to hook into Express and run before my own server logic, whereas I was looking for a library that I can call from my code, and trying to do something with its other than "hook it as Express middleware exactly as described" requires studying its code, which is headache-inducing callback hell.
Ideally the library should support passing custom data from the caller of authorize()
to the model. Currently I have to tuck it into objects like user
, which is hackish. For example, I want to record the IP from which the authorization request was made, but since the library doesn't give the model direct access to the request object, I have to pass it in the user object instead so saveAuthorizationCode
can read it.
Thank you @Maia-Everett for the help and explanation.
If you think there are valid use-cases for passing custom data beyond the current structures then feel free to open this as an idea in the discussion section as we are always looking to improve the library, while keeping it as unopinionated as possible.
In my implementation of an OAuth server using
node-oauth2-server
, I did it this way:
- The
/oauth/authorize
endpoint is implemented by the frontend, which takes the query parameters and calls an internal API via Ajax, which in turn callsauthorize()
to do all the validation before the consent screen is even shown.- The model's implementation of
saveAuthorizationCode
saves the generated authorization code in the database, marks the request as "needing user consent", and returns data needed to render the consent screen.- When the user clicks the approve button, the frontend calls another internal API endpoint to actually authorize the request. This endpoint loads the unconfirmed request, marks it as approved, and returns the redirect URI that
authorize()
would have returned to in the HTTPLocation
header.- The frontend redirects the browser using
window.location.replace
.Rube Goldberg would be proud. Unfortunately, this is the most unopinionated maintained implementation of OAuth2 in Node that I could find, and the most well-documented one.
oauth2orize
expects to hook into Express and run before my own server logic, whereas I was looking for a library that I can call from my code, and trying to do something with its other than "hook it as Express middleware exactly as described" requires studying its code, which is headache-inducing callback hell.Ideally the library should support passing custom data from the caller of
authorize()
to the model. Currently I have to tuck it into objects likeuser
, which is hackish. For example, I want to record the IP from which the authorization request was made, but since the library doesn't give the model direct access to the request object, I have to pass it in the user object instead sosaveAuthorizationCode
can read it.
This is precisely what I am doing also. Honestly, this method does not make any assumptions about the UI / server setup in any way and that is why I like it.
It seems AuthenticationHandler is to see who the user is from the request, not to show an additional dialog for explicit approval. Is it a supported flow?