Skip to content
This repository has been archived by the owner on Dec 10, 2023. It is now read-only.

JENKINS-52306 Improve SSO logout, continuous fix for JENKINS-11507 #25

Closed
wants to merge 1 commit into from

Conversation

gmshake
Copy link
Contributor

@gmshake gmshake commented Jul 2, 2018

Issue link JENKINS-52306

@martinspielmann martinspielmann added this to the 3.0.0 milestone Jul 13, 2018
@DuMaM
Copy link
Contributor

DuMaM commented Apr 10, 2022

@gmshake I know it was long time ago, but could you check if this working with latest master changes?
I would be grateful if you could add tests to it ;)

@gmshake
Copy link
Contributor Author

gmshake commented Apr 11, 2022

I'm glad to, but no bandwidths until next week.

@DuMaM
Copy link
Contributor

DuMaM commented Apr 11, 2022

No problem :)
Take your time. I was wondering if you can test it.
I dont have test env with sso yet, so I would be glad if you could test it out ;)
Your PR looks good.

@DuMaM
Copy link
Contributor

DuMaM commented Apr 22, 2022

@gmshake Thanks 😄
Did you tested it out after rebase?
Are you able to add some unit tests?
I do not have test env to check it out by myself for now.

@gmshake
Copy link
Contributor Author

gmshake commented Apr 24, 2022

No, it is not tested extensively yet. I'll report back later.

@DuMaM
Copy link
Contributor

DuMaM commented Oct 25, 2022

@gmshake Are you still interested in merging this PR?

Comment on lines +181 to +189
} else { // Auto login failed.
if (LOG.isLoggable(Level.FINE)) {
LOG.fine("User failed to log in");
}
HttpSession session = req.getSession(false);
if (session != null) {
session.invalidate();
}
SecurityContextHolder.clearContext();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only thing i'm missing here is cache clearing
If you invalidate user it still have records in cache which together with token caching can bypass sso

Copy link
Contributor Author

@gmshake gmshake Oct 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the only thing i'm missing here is cache clearing

I'm getting you here about the cache clearing.
Do you mean the crowd2-plugin caches tokens somewhere else other than in the session?

If you invalidate user it still have records in cache which together with token caching can bypass sso

Can you please elaborate this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean the crowd2-plugin caches tokens somewhere else other than in the session?

private CacheMap<String, User> userFromSSOTokenCache = null;

I think user should be removed from this cache in case of session termination.

Can you please elaborate this?

This will do the job
https://www.youtube.com/watch?v=LMgpuVKslw8

@gmshake
Copy link
Contributor Author

gmshake commented Oct 26, 2022

@gmshake Are you still interested in merging this PR?

Until the problem is fixed, or nobody complains about the problem ;)

@gmshake
Copy link
Contributor Author

gmshake commented Apr 18, 2023

@gmshake Are you still interested in merging this PR?

Until the problem is fixed, or nobody complains about the problem ;)

@DuMaM
I'll let you decide whether continue this or not.
I'm drowned in FreeBSD and have no bandwidth for this.

I'm sorry for that.

@DuMaM
Copy link
Contributor

DuMaM commented Apr 18, 2023

I will takeover this.

@DuMaM
Copy link
Contributor

DuMaM commented Aug 13, 2023

@akouznetchik could you help us with testing this?

@DuMaM DuMaM assigned DuMaM and akouznetchik and unassigned DuMaM Aug 16, 2023
@DuMaM
Copy link
Contributor

DuMaM commented Sep 28, 2023

@akouznetchik any updates?

@DuMaM DuMaM removed this from the 3.0.0 milestone Sep 28, 2023
@akouznetchik
Copy link
Contributor

@akouznetchik any updates?

Will take a look this weekend

@DuMaM
Copy link
Contributor

DuMaM commented Sep 29, 2023

@akouznetchik any updates?

Will take a look this weekend

Thanks man :)

@DuMaM DuMaM requested a review from a team October 10, 2023 20:07
@DuMaM
Copy link
Contributor

DuMaM commented Dec 10, 2023

Development of this plugin is stopped.

@DuMaM DuMaM closed this Dec 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants