Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix handling of Zope root users #412

Merged
merged 7 commits into from
Nov 6, 2017
Merged

Fix handling of Zope root users #412

merged 7 commits into from
Nov 6, 2017

Conversation

buchi
Copy link
Member

@buchi buchi commented Oct 21, 2017 •
edited
Loading

Another approach to fix #178, #127, #168
Obsoletes #378

  • The @login endpoints now checks for an installed JWT plugin in the acl_users folder where the logging in user has been found.
  • The JWT plugin is also installed in the Zope root acl_users folder.
  • Each JWT plugin uses a unique signing secret to prevent authenticating tokens issued by other JWT plugins.

@coveralls
Copy link

coveralls commented Oct 21, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.592% when pulling f2385cb on fix-root-login into 88a6779 on master.

@coveralls
Copy link

coveralls commented Oct 21, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.01%) to 96.549% when pulling 80aca9e on fix-root-login into 88a6779 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 96.549% when pulling 80aca9e on fix-root-login into 88a6779 on master.

@coveralls
Copy link

coveralls commented Oct 22, 2017 •
edited
Loading

Coverage Status

Coverage decreased (-0.01%) to 96.549% when pulling 9d34fc2 on fix-root-login into 88a6779 on master.

@buchi
Copy link
Member Author

buchi commented Oct 23, 2017

@tisto @sneridagh This one is ready for a final review. Can you check if it fixes your issues. I'm pretty sure it fixes #127 and #168, but I'm not sure I understand #178 correctly.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 96.335% when pulling c856c81 on fix-root-login into 88a6779 on master.

@coveralls
Copy link

coveralls commented Oct 23, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.593% when pulling 573a17c on fix-root-login into 88a6779 on master.

@tisto tisto requested a review from sneridagh October 23, 2017 17:44
@tisto
Copy link
Sponsor Member

tisto commented Oct 23, 2017

@buchi thank you! We will look into it during this week.

@tisto tisto self-requested a review October 25, 2017 18:13
tisto
tisto approved these changes Oct 25, 2017
View reviewed changes
Copy link
Sponsor Member

@tisto tisto left a comment

Choose a reason for hiding this comment

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

LGTM. @sneridagh could you check tomorrow if that fixes our issues? If that's the case go ahead and merge.

@coveralls
Copy link

coveralls commented Oct 25, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.593% when pulling 1a94de9 on fix-root-login into 7fa16c9 on master.

@coveralls
Copy link

coveralls commented Oct 30, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.667% when pulling 1c939da on fix-root-login into 8a84333 on master.

@coveralls
Copy link

coveralls commented Nov 3, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.667% when pulling 32a1a7b on fix-root-login into c9be8e3 on master.

@tisto tisto added this to the 1.0b1 milestone Nov 4, 2017
@buchi
Only login a user if we have a JWT plugin
ecc710d 
Do not login users that are found in an acl_users folder without a JWT plugin.
@buchi
Use a unique signing secret
914b836 
Make sure every JWT PAS plugin uses a uniqe signing secret by appending it's
path to the secret.
@buchi
Also install JWT plugin in root acl_users folder
86030f1
@buchi
Add upgrade step that installs PAS plugin in root acl_users folder
71325e1
@buchi
Add changelog entry for JWT fix
a44eed2
@buchi
Fix deprecation warning when decoding JWT token
290dc64 
Also refactor token decoding.
@coveralls
Copy link

coveralls commented Nov 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.67% when pulling 290dc64 on fix-root-login into 3aaf76e on master.

@coveralls
Copy link

coveralls commented Nov 6, 2017 •
edited
Loading

Coverage Status

Coverage increased (+0.03%) to 96.683% when pulling 037b85c on fix-root-login into 2b7ae2d on master.

sneridagh
sneridagh approved these changes Nov 6, 2017
View reviewed changes
Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

LGTM too! I also think it fixes #127 and #168. However, I think that #178 will still happen.

@sneridagh sneridagh merged commit 321ce09 into master Nov 6, 2017
@sneridagh sneridagh mentioned this pull request Nov 6, 2017
Open
@tisto tisto mentioned this pull request Nov 6, 2017
Closed
@lukasgraf lukasgraf mentioned this pull request Sep 13, 2018
Closed
@jensens jensens deleted the fix-root-login branch October 7, 2020 15:48
@mauritsvanrees mauritsvanrees mentioned this pull request Oct 19, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sneridagh sneridagh sneridagh approved these changes

@tisto tisto tisto approved these changes

Assignees
No one assigned
Labels
24 status: ready
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dual authentication (from cookie and token) issues in production environments
4 participants
@buchi @coveralls @tisto @sneridagh