Skip to content

Pass on direct query parameter during login - #17548

Merged
rullzer merged 1 commit into
masterfrom
fix/saml/pass_on_direct
Oct 16, 2019
Merged

Pass on direct query parameter during login#17548
rullzer merged 1 commit into
masterfrom
fix/saml/pass_on_direct

Conversation

@rullzer

Copy link
Copy Markdown
Member

If direct is set we should forward it again.
This to make SAML direct login work again.

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

@rullzer

Copy link
Copy Markdown
MemberAuthor

/backport to stable17

@blizzzblizzz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

looks good

@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 15, 2019
:value="OC.requestToken">
<input type="hidden"
name="direct"
:value="direct">

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
:value="direct">
:value="directLogin">

WHy not use directLogin directly?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

because the "direct" parameter is what we use in various places at the moment, including SAML and GS

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah, but it's coming as a prop, so we don't care about the name of the variable :)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

@skjnldsv you are right

@skjnldsvskjnldsv left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See comment

@schiessle

Copy link
Copy Markdown
Member

tested it successfully!

@schiessle

Copy link
Copy Markdown
Member

would be great to have this in 17.0.1. I know some users waiting for it.

@rullzer

Copy link
Copy Markdown
MemberAuthor

It will be backported yes

@rullzer

Copy link
Copy Markdown
MemberAuthor

Ok so i changed the code a bit.
Not direct is only set (and send) if you set direct=1 in the query. Which mimicks more the original behavior.

If direct is set we should forward it again.
This to make SAML direct login work again.
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
rullzerforce-pushed the fix/saml/pass_on_direct branch from a373b07 to 38730b4CompareOctober 16, 2019 21:25
@rullzer
rullzer merged commit a122fe6 into masterOct 16, 2019
@rullzer
rullzer deleted the fix/saml/pass_on_direct branch October 16, 2019 22:24
@backportbot-nextcloud

Copy link
Copy Markdown

The backport to stable17 failed. Please do this backport manually.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4. to releaseReady to be released and/or waiting for tests to finishbug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@rullzer@schiessle@ChristophWurst@blizzz@skjnldsv