fix infinite redirect by clean up expired session

[galupa]

Hi,

This should fix infinite redirect problem when session lifetime is set up on Redmine.
From what I see the problem is after the session is expired, Redmine will redirect to login page because it detects the session is expired and on the login page it also check whether the client have any session and check if the session is expired or not, if it is it will redirect to login page and it will repeat it infinitely.
By default Redmine is clean up session if there any changes on user, in this case when the session is expired, Redmine should set the user as null which will reset the session, however since this plugin overwrite the logged_user, it does not clean up the session, that is why the expired user session will keep lingering and it cause the redirection.

What I do in this PR is just clean up the session when Redmine detect the session is expired (it will call require_login) and also call the default logged_user if the logged user is not ioc user. The second one is not mandatory but it can be a good guard if the system allow both oic user and Redmine internal user.

Please check it.

Best Regards,
Rangga

[iWangJiaxiang]

Hope this would be merged ASAP

[eni23]

Any updates? Wold be really really nice if this will be merged

[electrical]

I've implemented this but still hit the same infinite redirect :-(
Unsure how to debug it further

[galupa]

I've implemented this but still hit the same infinite redirect :-( Unsure how to debug it further

Have you clean up the browser cache when testing this? It is possible some cookies / session from previous version is still stored on the browser.

[electrical]

Not sure if my users did that. Will check with them.

[ataraxus]

this hits me also all the time... will try this too and report back if it works

[ataraxus]

Hey Guys, after having used this a couple of months in production, have to say it doesnt solve the issue :(
Still getting infinite redirect after expired session.

[galupa]

Hi @ataraxus ,

Thank you for checking, is it possible for you to provide some logs? We have use the patch for about a year now and we have not got any issue, but if you can provide some logs that will be great, I can do some checks.

Best Regards,

[electrical]

I've been running into the same issues like @ataraxus
Next time it happens will try to grab as much info as possible.

[electrical]

What I've found so far:

A user gets redirected to our OpenID server due to Filter chain halted as :session_expiration rendered or redirected
A new entry gets created in the oic_sessions table.
This entry contains state,nonce, created_at and updated_at information but everything else is empty.
The OpenID server wants to redirect the user back to Redmine because the user has a valid session with the OpenID server.
And it starts all over again.

Wed, Jun 15 2022 10:01:58 am | Redirected to https://redmine.corp.com/oic/login
Wed, Jun 15 2022 10:01:58 am | Filter chain halted as :session_expiration rendered or redirected
Wed, Jun 15 2022 10:01:58 am | Completed 302 Found in 7ms (ActiveRecord: 2.1ms)
Wed, Jun 15 2022 10:01:58 am | Redirected to https://auth.corp.com/auth/realms/corp/protocol/openid-connect/auth?client_id=redmine&nonce=4476da00-8da6-4d5d-be00-f89d06514ed8&redirect_uri=https%3A%2F%2Fredmine.corp.com%2Foic%2Flocal_login&response_type=code&scope=openid+profile&state=c6f5f45f-1e89-42b5-b835-096d2a932580
Wed, Jun 15 2022 10:01:58 am | Completed 302 Found in 95ms (ActiveRecord: 90.0ms)
Wed, Jun 15 2022 10:01:59 am | Started GET "/oic/login" for 127.0.0.1 at 2022-06-15 09:01:58 +0000
Wed, Jun 15 2022 10:01:59 am | Processing by AccountController#oic_login as HTML
Wed, Jun 15 2022 10:01:59 am | Redirected to https://redmine.corp.com/oic/login

[galupa]

Hi @electrical ,

Thank you for the logs, can I also know your plugin config also? I just want to know if login_selector is on or off.

Cheers

[electrical]

@galupa config:

--- !ruby/hash:ActiveSupport::HashWithIndifferentAccess
enabled: 'false'
client_id: redmine
openid_connect_server_url: https://auth.corp.com/auth/realms/corp
client_secret: xxxxxxxxxxxxxxxxxxxxxxx
scopes: openid,profile
group: Users
admin_group: RedmineAdmin
dynamic_config_expiry: ''
create_user_if_not_exists: 'true'

Interestingly enough the plugin saves things in a weird way. 'false' is still true :)

[electrical]

@galupa do you have any ideas on this? Thanks!

[galupa]

Hi,

Apologies, I have been busy lately, but I think I know what the problem is, I will take a look at it some times this week and will update.

Cheers

[electrical]

Wonderful. thank you!

[galupa]

Hi @electrical ,

I have made some changes, looks like the previous code is working because we are enabling the login selector, and my previous fix is fail if the login selector is disabled.
I have made some adjustment so that the fix will work on both scenario.

I have do some testing and it should work now but please check it if you have some time, and let me know if the problem still happening.

Best Regards,
Rangga

[ataraxus]

LGTM

[guoanhao]

Thank you. Which version of redmine is applicable

[galupa]

I am using version 4.1.x at the moment, I think it should be still compatible on version 4.2.x but I have not properly tested it against it.
And most likely will not working on version 5 because of Zeitwerk replacing classic autoloader on rails 6.

[galupa]

Actually there is already a PR for Redmine 5 so it should work on Redmine 5 now.

[galupa]

Hi All, does anyone knows who can Merge this PR?

[guoanhao]

I am using version 4.1.x at the moment, I think it should be still compatible on version 4.2.x but I have not properly tested it against it. And most likely will not working on version 5 because of Zeitwerk replacing classic autoloader on rails 6.

thank you，My report is wrong，Have you ever met

[galupa]

I am using version 4.1.x at the moment, I think it should be still compatible on version 4.2.x but I have not properly tested it against it. And most likely will not working on version 5 because of Zeitwerk replacing classic autoloader on rails 6.

thank you，My report is wrong，Have you ever met

Have you tried check the Create user if not exists option?
Btw, if you have a question not related to PR maybe better create an issue instead so that the others does not get notification for non related question.
I am happy to answer another question in issues in my fork, so feel free to do that.

Cheers

[guoanhao]

I am using version 4.1.x at the moment, I think it should be still compatible on version 4.2.x but I have not properly tested it against it. And most likely will not working on version 5 because of Zeitwerk replacing classic autoloader on rails 6.

thank you，My report is wrong，Have you ever met

Have you tried check the Create user if not exists option? Btw, if you have a question not related to PR maybe better create an issue instead so that the others does not get notification for non related question. I am happy to answer another question in issues in my fork, so feel free to do that.

Cheers

thank you，i Created a new issue：#77

[ndkhaivn]

Hi, is there anything blocking this from merging?

[ericbrun-73]

Hi,
We have this problem (Too many redirect) on our Redmine 5.0.5.
I patched the Open ID Connect plugin with the correction.
We testing now ....

I hope that fix our Redmine access ....
We will see ....

[mattock]

We seems to have the same redirect loop issue, but it is not triggered by session expiration:

Filter chain halted as :check_if_login_required rendered or redirected

In our case session expiration was not enabled, so something else is triggering this.

[luizjr]

@galupa , I tested the code and it worked as expected. I still don't know why not to do the merge

[deather]

Hi, Same problem here.

I apply the patch on a browser that is encountered the problem. After applying everithing is working like a charm.

Thank you @galupa

[mattock]

We seems to have the same redirect loop issue, but it is not triggered by session expiration:

Filter chain halted as :check_if_login_required rendered or redirected

In our case session expiration was not enabled, so something else is triggering this.

We applied this PR and the problem is gone. We know that with some browsers and/or operating systems the problem does not manifest itself much. For example, MacOS with Safari (I believe) is not affected as much as Fedora Linux 38 with Firefox.

In any case this PR solves a genuine issue that affects many. So far I have not seen any issues with this fix.