Edit returning 404

[des-des]

Edit didn't work and it didn't say why it didn't work it just gave a 404.

EPIC: #90

[Karyum]

Hmm i ran into this today and tried to fix it for a whole 2 hours, it's something weird happening to the referredUrl cookie where the id of the event becomes undefined and it tries to go to /en/edit-event/undefined, i'll try further more and see what comes up

[Karyum]

I have managed to reproduce this behavior only twice, and still not sure what's causing it.

• one way to reproduce it is to delete the token cookie while in the edit form and then submitting it.
• the second way which only saw it happened twice only once with me and once when Liwan were user testing it 😅 , and it happened always after we added an event and then we wanted to edit it again and after that we would get the 404, but not sure whats going on cause im trying to make it happen again without any success only worked once

[m4v15]

Hi 👋 I was bored and thought I'd look at this - is it the OTP giving the 404 or the Data Entry app? The only place that data entry sends a 404 is in the lang middleware here. All of the other errors are 400 or 500 I think.

Also, could be something to do with this cookie expiring after 5 minutes possibly?

[m4v15]

I lied, Data Entry could also potentially send a 404 here if it gets a 404 back from the OTP.

Good luck, this seems tough.

[des-des]

Can you put a request logger in otp and also data entry

[Karyum]

@m4v15 sorry my bad it was a 400, the API would respond with a 401 ( Unauthorized ), not sure why cause i am a SUPERUSER and only when i remove the cookie and log in again it would work any idea?

Can you put a request logger in otp and also data entry

@des-des
I'm guessing this direct it at me ? and do you mean while debugging it locally or make a PR?

[m4v15]

@Karyum maybe your Auth Code has expired on the OTP end? Not sure what the length of them are set to, will have a look at the OTP OAuth stuff later if I get a chance

[m4v15]

I think the access token's expire after an hour, so possibly if your are logged in for over an hour with the same code you will get that? I should really do some real work

[Karyum]

I think the access token's expire after an hour, so possibly if your are logged in for over an hour with the same code you will get that? I should really do some real work

Yeah i guess that would be it, @des-des maybe a solution for that would be to have a refresh login in the nav-bar, thoughts ?

[des-des]

@Karyum @m4v15 Finding it quite hard to dig around all the code here.

The lifetime of the access token is indeed one hour: https://oauth2-server.readthedocs.io/en/latest/api/oauth2-server.html#token-request-response-options-callback.

See the default time on the access token. The correct thing to do here is to use the refresh token to generate a new access token. @m4v15 if I am correct, we have not added this functionality

Good solution

1. Make sure data entry gracefully deals with the problem as we see it now. Ie redirects to login page properly clears the session, etc, when authentication fails.
2. Implement using the refresh token in OTP
3. Detect expired tokens in data-entry, and refresh the token without involving the web client.

@Karyum this is not an easy task, if you fancy doing something hard, that involves reading lots of code + api docs, you should give this a go! ✨

@Karyum FYI
https://github.com/foundersandcoders/open-tourism-platform/blob/master/src/controllers/oauth.js
https://github.com/oauthjs/express-oauth-server
uses this under the hood:
https://oauth2-server.readthedocs.io/en/latest/index.html

[m4v15]

@des-des yeah, refresh token functionality isn't set up, I don't think it's too much more that actually needs to be implemented in order for it to work, but yeah this:

involves reading lots of code + api docs

is accurate. I kind of had an understanding of it at one point and nearly got it working, but I was deep in OAuth land at the time.

Also, have you confirmed it's the access token expiring that's causing the bug?

Finding it quite hard to dig around all the code here.

Yeah to be honest I think we took the wrong route with the authentication stuff here and it could do with a bit of a refactor, it's really hard to follow when re-reading it.

[Karyum]

@des-des just to be clear the way we should deal with it right now is to remove the token and redirect the user to the login page so he can get a new one correct ?

also @m4v15 there is a branch in the OTP called refreshTry is this where you were trying to implement the refresh token feature

[m4v15]

Lol, I imagine it is, yes

[Karyum]

i just noticed the md link screw up 😅

[Karyum]

So after looking into the OTP code for a while, and try stuff even if i change the expiration time of the cookie on the OTP ( locally ) to 5 seconds still takes an hour for the token to not work anymore. also i found out the data entry was over writing the the OTP's cookie, cause both of them were called token but somehow it was still working this entire time not sure how any idea about this ?

the solution that i thought of for the data entry to deal with the 404 unauthorized issue is to send the user back to OTP's login page for now does that sound good to you for now @des-des ?

oh also what about extending the cookie's expiration date to more than an hour ? something like 4 hours cause it makes more sense doesn't it ?

[m4v15]

Err I'm sure @des-des can be more clear on this but pretty sure as those cookie's will be from different servers they won't be overwritten or anything - otherwise any server could accidentally overwrite the cookie from any other server.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Cookies

[des-des]

@Karyum

There are two authentication methods on OTP. The one that uses cookies you are not interested in.

1. With the cookie. When a user authorises an app to use opt, they have to log in directly to otp. This is where the cookie is used on OTP.

2. With an authorisation header. This is how other backends are expected to interact, and how data-entry interacts (i think)

@Karyum you are correct about the alternative solution, I left it out.

Bad Solution

1. Make sure data entry properly redirects back to log-in page if the session is invalid
2. We could also increase the token exp time to 1 week (for now). See my previous solution for how to do that

[Karyum]

@des-des

After looking through the OTP's code for a while and checking out oauth2-server docs, I have a pretty good idea on how to implement the refresh token functionality to the API and how to deal with it in a better way in the data entry. first of all thanks @m4v15 for giving me a head start by creating this branch 😄 .

now in order for this to work these changes need to happen:

• When the data entry gets an Unauthorized error from the OTP, it will use the refresh_token from the stored cookie to make a request for a refreshed access token on /oauth/token with a grant of "refresh_token", by doing that the user won't even feel what happend 😬 ( might just take longer by a sec ).
• In the OTP API the only change that i can see ( other than what @m4v15 did 😅 which also includes testing ) that needs to be done is whenever a client is created a grant type of "refresh_token" needs to be added to the array of grants.

I only have 2 small questions:

• Is the whole reason we are populating here with the user and client is for having some sort of history of who logged and and from which app?
• Also how can we add the grant "refresh_token" to the grants array for the data-entry ( which is on the production database ) ? cause there is no way to update a client in the API yet, do i just manipulate the database straight away somehow 😬 ?

[mattlub]

@Karyum

Is the whole reason we are populating here with the user and client is for having some sort of history of who logged and and from which app?

I think we are just doing that because the docs say we should :D

[des-des]

@Karyum On your first question, I am not 100% sure. But the functions defined there are used internally by the oauth module, so I will go with lubs and say that its what the docs told us to do.

Question 2: You are right to say we do not have functionality to change the grants of a currently existing client. First, manipulate the db directly to add this, second, make sure new clients get given this automatically!

[Karyum]

so what i need to do is:

• manipulate the production database to add the refresh_token to the grants array.
• add the testing for the refresh token functionality. [awaiting review]
• Date entry to handle the Unauthorized, by requesting for the refresh token from the API. [awaiting review]
• Add an update route to the api which would update the client.

[des-des]

Wikid: Here is some more specs: http://oauth2-server.readthedocs.io/en/latest/model/overview.html?highlight=grant#refresh-token-grant

1. The only changes you need to make to the live db is for existing clients (ie data-entry) https://github.com/foundersandcoders/open-tourism-platform/blob/master/src/models/auth/Client.js#L8

2. When a new client is created, you should also give it a refresh token grant by default
   https://github.com/foundersandcoders/open-tourism-platform/blob/master/src/controllers/oauthClient.js#L23

[Karyum]

@des-des do you think we still should extend the token's expiry date or keep it an hour since we are implementing the refresh token functionality now?

[des-des]

@Karyum if we have the refresh token no need to extend the exp