Fix SSO SAML redirection not including query string

When a user without a SSO session attempted to access anything in a SAML group with a query string, previously GitLab would redirect the user back to original path but drop the query string.

We fixed the redirection originally in !66791 (merged), but request.path_info drops query strings. To ensure the query strings are passed to the RelayState, we need to use request.fullpath.

Relates to #338833 (closed)

assigned to @stanhu

changed the description

changed milestone to %14.3

added groupauthentication and authorization [DEPRECATED] typebug labels

requested review from @dblessing

added backend label

	1 Warning
	This MR changes code in ee/, but its Changelog commit is missing the EE: true trailer. Consider adding it to your Changelog commits.

Reviewer roulette

Changes that require review have been detected!

Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:

Category	Reviewer	Maintainer
backend	George Koltsov (@georgekoltsov) (UTC+1, 8 hours ahead of @stanhu)	Shinya Maeda (@shinya.maeda) (UTC+7, 14 hours ahead of @stanhu)
test Quality for spec/features/*	Andrejs Cunskis (@acunskis) (UTC+3, 10 hours ahead of @stanhu)	Maintainer review is optional for test Quality for spec/features/*

To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.

To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.

Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.

If needed, you can retry the danger-review job that generated this comment.

Generated by Danger

added 1 commit

• e94b0d31 - Fix SSO SAML redirection not including query string

Compare with previous version

resolved all threads

added 1 commit

• 25df7c34 - Fix typo

Compare with previous version

resolved all threads

added 1 commit

• 81764ae4 - Fix typo

Compare with previous version

added 9 commits

• 81764ae4...1dae49eb - 8 commits from branch master
• 4ede4460 - Fix SSO SAML redirection not including query string

Compare with previous version

Setting label(s) ~"Category:Authentication and Authorization" devopsmanage sectiondev based on ~"group::access".

added devopsmanage sectiondev + 1 deleted label

@stanhu, please can you answer the question: Should this have a feature flag? to help with code review for the Access group.

This nudge was added by this triage-ops policy.

resolved all threads

requested review from @mksionek and removed review request for @dblessing

Tested locally, working as expected. Approving.

@mwoolf can you take a look?

approved this merge request

removed review request for @mksionek

requested review from @mwoolf

@mksionek, thanks for approving this merge request.

This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.

For more info, please refer to the following links:

• rspec minimal jobs
• merging a merge request.

resolved all threads

approved this merge request

enabled an automatic merge when the pipeline for 566d6dcb succeeds

merged

mentioned in commit 0d365ec3

Thanks @stanhu

added workflowstaging label

added workflowcanary label and removed workflowstaging label

added workflowproduction label and removed workflowcanary label

mentioned in issue #338833 (closed)

added releasedcandidate label

added releasedpublished label and removed releasedcandidate label

mentioned in merge request kubitus-project/kubitus-installer!236 (merged)