slavab89 / oauth2-server-example-mongodb

Example for using node-oauth2-server with a mongodb backend
MIT License
40 stars 10 forks source link

Question #3

Closed HenrikGr closed 6 years ago

HenrikGr commented 6 years ago

Hi,

I have played around to learn more about oauth and used you repo as an inspiration. First I want to say thank you for this and it has been helpful for me. However, of course I have some question I hope you could help me with?

I can play around and get it to work except where I need to base64 encode the clientID and clientSectrets.

I'm using the online tool https://www.base64encode.org/ without success.

Do you know what it could be? :-)

slavab89 commented 6 years ago

Hello 😄 Glad i could be of help. Although i havent updated this example to the latest 3.0 official version, i believe it should still work and the changes would be minimal.

The clientId and secret should be in the form of id:secret and then you should do the base64 encode on them So for example for abcde:1234 you would get YWJjZGU6MTIzNA==

Whats the request that you're trying to do?

HenrikGr commented 6 years ago

Hi,

I have problem with the refresh token request.

If I understand the flow correct.

  1. Run the password request and that gives me both an access and an refresh token.
  2. I'm trying to use the refresh token request and get a failure saying "refresh token is invalid".

I suppose I should use the value of the recievied refresh token from password request and then ask for the refresh token to get a new access token?

To sort out stuffs I'm sure about. Base64 encoding - check. How to do the request - check.

But i strongly suspect there is a bug somewhere in the model. It breaks at this code in the refresh_token_grant_type.js

 if (token.client.id !== client.id) {
    throw new InvalidGrantError('Invalid grant: refresh token is invalid');
  }

When I look in the model code function getrefreshToken I can see this.

const extendedClient = Object.assign(dbToken.client, { id: dbToken.client._id }); const value = Object.assign(dbToken, { client: extendedClient });

We extend the return value with an id prop and that lead me to think we may need to do that in the function saveToken as well.

I have spend to much time ripping my hair out and was going to put this on hold for a while but if you know what it could be I'm all ears.

HenrikGr commented 6 years ago

I will get back to you when I have figured it out. I think there is some minor flaws in the models and especially when it comes to how we returning that client.id that according to the docs should be a string.

I do not understand that property 100%. It is not in my mongoose model and not in yours and I suppose it is not persisted anywhere, it is just an id we need to pass around in our model.

slavab89 commented 6 years ago

I've looked a bit at the code. I believe that the issue that you're facing is most likely that you're trying to create 2 new access token from the same refresh token. Meaning: You get an access + refresh tokens from the password grant. Then, when you try to use the refresh token with a refresh_token grant you get a new access token, but when you're trying to do that again, you get the error of refresh token is invalid.

This is the default behavior as can be seen here - every time you use the refresh token to get a new access token, it is revoked. It is mainly done for security reasons.

If you would like to use the same refresh token multiple times you can pass { alwaysIssueNewRefreshToken: false } option when creating the token at the token middleware

Let me know if that helps you in any way.

HenrikGr commented 6 years ago

Hi,

Thank you for trying to help me. I have it working now and you could se my model implementation here: https://github.com/HenrikGr/hgc-auth/blob/master/server/oath2/model.js

I do not remember how I got it to work but I'm using version 3.0 and it started to work when I changed the return object in the getClient model function. If I remember there was some inconsistency in regards to the id prop returned. Original it returned the id value from the client id / secret and not the actual _id string value from mongo.

I'm also using mongoose 4.5.9 and use the built in virtual prop .id to get the bson object value as a string.

PS. client_credentials, password and refresh_token working now and I'm going to implement the rest later.

I would be glad if you could do a little code evaluation on my model. :-)

If not, thank you for everything.

HenrikGr commented 6 years ago

Ohh, one more thing. In order to use the built in virtual getter to get the _id value as a string, I could not use lean().

slavab89 commented 6 years ago

You should add exec() to all places that do a mongoose query, i have forgot to add it in my code as well. Check all the examples in mongoose site http://mongoosejs.com/docs/4.x/docs/promises.html

Moreover, yes its true that you can use virtual getters once you do lean() its intentional in mongoose.

I'll close this issue as it seems to be resolved for you, but lets continue speaking if needed :)

HenrikGr commented 6 years ago

:-)

One last question.

In our model, you return this in the getClient function: .then(client => (client ? Object.assign(client, { id: clientId }) : null))

Is this a bug? That is not the id from det mongose document, it is the clientId?

Just want to sort that out since that was the change I did.

slavab89 commented 6 years ago

Its not a bug, its intentional :) All the logics in the node-oauth2-server work with id on the client. When making validations here or here for example This is done this way so that the library wont be mongo specific and you'll be able to work with it using any DB you want

HenrikGr commented 6 years ago

Ok, thanks for clarify this for me. When I changed to use the mongo id string the refresh_token started to work. The validation you have in your first link was where it failed.

HenrikGr commented 6 years ago

I tried to return the clientId as id prop from the getClient model function and now it fails again on the refresh_token.

HenrikGr commented 6 years ago

Dont spend time on this. :-) I'm sorting it out. There is many new things I'm learning and mongo is complicated. Knowing to use lean() for example, it works fine now. :-)

Should have stuck with your code from the beginning. LOL. Thanks and have a nice sunday.

slavab89 commented 6 years ago

It you need any help, dont hesitate to ask :)

If you see the error about refresh token is invalid once again, check if it happens in the following flow:

  1. You do a request using password grant type and get an access_token and refresh_token
  2. You do a request using refresh_token grant type and get a new access_token
  3. You do another request using refresh_token grant type with the refresh token from step 1. <-- Error happens in this step

If this is the flow and you see the error then check my comment about re-using the same refresh token for multiple requests.

HenrikGr commented 6 years ago

I do understand the flows and it works well.

  1. password request --> access_token AND refresh_token. The refresh_token should be used to issue a new access_token, preferable when the access_token is going to expire.
  2. Use the refresh_token from above to get a new access_token --> give a new result as above. A new access_token and refresh_token has been created and persisted AND the previous refresh_token has been revoked (deleted from mongo).

All this works for me now including the client credential grant.

My problem was down to that I mixed up when to use lean() or not with Object.assign. It did not work to extend the client object with id prop when I did not use lean().

HenrikGr commented 6 years ago

My conclusion is that I can not extend a mongoose document with Object.assign, it must be a plain javascript object.

slavab89 commented 6 years ago

Correct. If you want to use virtual properties then you can return the mongoose document and then transform it to a plain object using toObject with virtuals Check http://mongoosejs.com/docs/4.x/docs/api.html#document_Document-toObject

btw, on a side note, if you try to do delete mongooseDocument.someProperty it will also fail cause mongoose document is not a plain JS object, so you cant delete stuff from it :)

HenrikGr commented 6 years ago

Yes, when I thought I got it working I was using the transform toObject but as we already know, I was using the wrong client id. :-)

Anyway, I have notice you are using loadash _.assign in the saveToken function. I do not do that, I'm using Object.assign and it seems to work anyway.

Do you remember why you did that?

slavab89 commented 6 years ago

Unfortunately no. I see no real reason to use _.assign there in fact.