Allow using an app token to login with v2 flow auth#29531
Conversation
juliusknorr
force-pushed
the
bugfix/noid/flow-auth-v2-apptoken
branch
from
f12a745 to
558f6e0
Compare
juliusknorr
requested review from
a team,
ArtificialOwl,
CarlSchwan,
blizzz,
skjnldsv and
tobiasKaminsky
and removed request for
a team
| /** | ||
| * @PublicPage | ||
| */ | ||
| public function apptokenRedirect(string $stateToken, string $user, string $password) { |
There was a problem hiding this comment.
this is identical to the same method of ClientFlowLoginController.php, innit? Maybe moving this into a trait to avoid duplication?
There was a problem hiding this comment.
The only difference there is the method call to validate the sate token which is isValidStateToken vs isValidToken, but we can adjust those of course. Moving that into a trait makes sense 👍
There was a problem hiding this comment.
OK, so it requires some more restructuring and actually going for a base controller class seems more suitable, though in order to keep this a smaller backportable change I'd rather do this in a follow up for master only. What do you think? I'd push the refactoring then to a separate PR.
There was a problem hiding this comment.
OK, so it requires some more restructuring and actually going for a base controller class seems more suitable, though in order to keep this a smaller backportable change I'd rather do this in a follow up for master only. What do you think? I'd push the refactoring then to a separate PR.
Sane assessment, I agree with you.
|
Needs some further work since when using it in the client we'll need to update the poll result on v2 instead of performing the redirect |
juliusknorr
added
2. developing
juliusknorr
force-pushed
the
bugfix/noid/flow-auth-v2-apptoken
branch
from
558f6e0 to
ced51d7
Compare
|
Design-wise, instead of that separate section, what would be a bit more obvious is the text/link "Alternative login using app token" in the same big container, below the big "Log in" button, centered, and underlined. |
Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
juliusknorr
force-pushed
the
bugfix/noid/flow-auth-v2-apptoken
branch
from
ced51d7 to
cdda25a
Compare
|
@jancborchardt Updated:
|
juliusknorr
added
3. to review
4 participants
The possibility to authenticate using an existing app token was only present in v1 flow auth, however there are use cases for it with v2 as well, e.g. when SAML is setup as primary login method but there are some local users that might need to login with the desktop client.
This is the button that this is adding for v2: