Page MenuHomePhorge
Differential D5074

Token validation when providing it as a password
ClosedPublic
Actions

Authored by mollekopf on Thu, Jan 2, 12:00 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Jan 24, 4:00 AM
Unknown Object (File)
Thu, Jan 16, 9:36 AM
Unknown Object (File)
Thu, Jan 16, 4:37 AM
Unknown Object (File)
Thu, Jan 16, 4:26 AM
Unknown Object (File)
Thu, Jan 16, 4:21 AM
Unknown Object (File)
Sat, Jan 11, 7:53 AM
Unknown Object (File)
Fri, Jan 10, 9:27 AM
Unknown Object (File)
Thu, Jan 9, 4:05 PM
View All 22 Files
Subscribers
machniak

Details

Reviewers
machniak
Group Reviewers
Restricted Project
Commits
rK77ced2efb642: Token validation when providing it as a password
Summary

We rely on this because the roundcube files plugin uses the auth/login
route to obtain a token using the password, which is a different token
in case of SSO.

Diff Detail

Repository
rK kolab
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

mollekopf requested review of this revision.Thu, Jan 2, 12:00 AM
mollekopf created this revision.
Harbormaster completed remote builds in B52550: Diff 14536.Thu, Jan 2, 12:00 AM
mollekopf added a reviewer: Restricted Project.Thu, Jan 2, 12:02 AM
machniak requested changes to this revision.Thu, Jan 2, 8:56 AM
machniak subscribed.

Placing this in User::validateCredentials() maybe is not a bad idea, but it creates some redundancy. See NGINXController::authorizeRequest() - AuthUtils::tokenValidate() now will be called twice. Also, test this new case in Unit/UserTest::testPasswordValidation().

This revision now requires changes to proceed.Thu, Jan 2, 8:56 AM
mollekopf updated this revision to Diff 14546.Mon, Jan 6, 9:34 AM

Addressed comments

Harbormaster completed remote builds in B52562: Diff 14546.Mon, Jan 6, 9:35 AM
machniak requested changes to this revision.Mon, Jan 6, 10:27 AM
machniak added inline comments.
src/tests/Unit/UserTest.php
69 ↗(On Diff #14546)

It should be assertTrue(), right? A test for "valid token of another user" would be nice too.

This revision now requires changes to proceed.Mon, Jan 6, 10:27 AM
mollekopf updated this revision to Diff 14548.Mon, Jan 6, 11:10 AM

Now with an actual test

Harbormaster completed remote builds in B52564: Diff 14548.Mon, Jan 6, 11:10 AM
mollekopf updated this revision to Diff 14550.Mon, Jan 6, 11:11 AM

Comment style

Harbormaster completed remote builds in B52566: Diff 14550.Mon, Jan 6, 11:11 AM
machniak requested changes to this revision.Mon, Jan 6, 11:34 AM

After another look I think we might be doing something wrong here. A valid token should disable/skip 2FA use. Current User::findAndAuthenticate() will not skip 2FA if the password is a valid token.

This revision now requires changes to proceed.Mon, Jan 6, 11:34 AM
mollekopf updated this revision to Diff 14552.Mon, Jan 6, 12:24 PM

Moved the token validation to findAndAuthenticate, so we can skip mfa when a token has been validated.

Harbormaster completed remote builds in B52568: Diff 14552.Mon, Jan 6, 12:24 PM
machniak accepted this revision.Mon, Jan 6, 12:35 PM
This revision is now accepted and ready to land.Mon, Jan 6, 12:35 PM
Closed by commit rK77ced2efb642: Token validation when providing it as a password (authored by mollekopf). · Explain WhyMon, Jan 6, 12:42 PM
This revision was automatically updated to reflect the committed changes.
mollekopf added a commit: rK77ced2efb642: Token validation when providing it as a password.

Revision Contents
Changeset List

PathSize
src/
app/
Http/
Controllers/
API/
V4/
9 lines
19 lines
tests/
Feature/
18 lines

Diff 14554

View Options

src/app/Http/Controllers/API/V4/NGINXController.php

Loading...
View Options

src/app/User.php

Loading...
View Options

src/tests/Feature/UserTest.php

Loading...