Add possibility to encode the state param as UrlEncodedBase64 #2312

brezinajn · 2023-11-07T12:23:50Z

Description

This is continuation of #2239

Added a feature to optionally Base64Url encode the state param.
The feature is behind a configuration flag, so the default behavior remains unchanged.

Motivation and Context

We're facing an issue where the state parameter gets truncated by the provider, leading to an incorrect application state after login. It's probably this issue but in our case with an oidc provider.

How Has This Been Tested?

We used a build from this commit as a drop-in replacement for 7.5.0 on our testing environment. Works as expected.

Checklist:

My change requires a change to the documentation or CHANGELOG.
I have updated the documentation/CHANGELOG accordingly.
I have created a feature (non-master) branch for my PR.
I have written tests for my code changes.

JoelSpeed

I'm ok with this. I think though when we go through the alpha config migration to GA, I would probably suggest changing up the bool to an enum.

Something like stateEncoding: Plain | Base64 would be more flexible and allow us to add additional options in the future should we choose to do so.

JoelSpeed · 2023-11-25T11:19:08Z

Looks like there's a conflict that will need to be resolved before we merge

brezinajn · 2023-11-27T09:40:07Z

Looks like there's a conflict that will need to be resolved before we merge

Resolved

tuunit · 2023-12-03T09:23:03Z

I'm ok with this. I think though when we go through the alpha config migration to GA, I would probably suggest changing up the bool to an enum.

Something like stateEncoding: Plain | Base64 would be more flexible and allow us to add additional options in the future should we choose to do so.

I'll take this over.

JoelSpeed · 2023-12-03T10:53:51Z

Can we fix the linter issue please?

tuunit · 2023-12-17T08:54:04Z

Hi @brezinajn,

Hope you are doing well. We would like to get this merged. The following linting issue is holding us back:

Error: oauthproxy.go:1205:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
	} else {
		return rawString
	}

oauthproxy.go

Co-authored-by: Jan Larwig <jan@larwig.com>

brezinajn · 2023-12-19T13:47:58Z

Hi @brezinajn,

Hope you are doing well. We would like to get this merged. The following linting issue is holding us back:
Error: oauthproxy.go:1205:9: indent-error-flow: if block ends with a return statement, so drop this else and outdent its block (revive)
	} else {
		return rawString
	}

Hi, sorry I'm on my vacation, so I'm not at my PC currently. Committed your suggestion. Hope it resolves the issue. Should there be another issue, please let me know. Thanks

oshmyrko · 2024-01-18T17:12:31Z

Hi guys! Any updates on this?

kvanzuijlen · 2024-01-18T17:29:46Z

Hi guys! Any updates on this?

This should be picked up during the next round of merging hopefully.

* enhancement: Change base image from alpine to distroless (oauth2-proxy#2295) * Changed base image from alpine to distroless * chore: updated Makefile * fix: removed arm/v6 and ppc64le for distroless variant * Update Dockerfile * Update Makefile * docs: Add README-section, CHANGELOG-entry and --pull to prevent caching --------- Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Add possibility to encode the state param as UrlEncodedBase64 (oauth2-proxy#2312) * Add possibility to encode the state param as UrlEncodedBase64 * Update CHANGELOG.md * Update oauthproxy.go Co-authored-by: Jan Larwig <jan@larwig.com> --------- Co-authored-by: Jan Larwig <jan@larwig.com> * NGINX return 403 for sign_in (oauth2-proxy#2322) (oauth2-proxy#2323) Co-authored-by: Sven Ertel <sven.ertel@bayernwerk.de> * chore: Create sha256sum for tar instead of binary (oauth2-proxy#2343) * Create sha256sum for tar instead of binary * chore: Add checksum for binary * chore: Updated changelog --------- Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Log error details when failed loading CSRF cookie (oauth2-proxy#2345) * Log error details when failed loading CSRF cookie * Add a record about this PR to CHANGELOG.md --------- Co-authored-by: Ondrej Charvat <ondrej.charvat@yunextraffic.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * Feature - Add env variable support for alpha struct (oauth2-proxy#2375) * added envsubstring package and added simple test cases.imple tests. * added documentation * added changelog entry * added documentation to wrong file . * changed tests to ginkgo format * update project to use better maintained library * use defer to clear test variable after tests finished * updated docs for the new package documentation and fixed bad english * refactored function to "reduce" complexity. * updated changelog for new version updated readme * minor formatting --------- Co-authored-by: Haydn Evans <h.evans@douglas.de> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * remove nsswitch workaround (oauth2-proxy#2371) Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * feat: Added renovate configuration (oauth2-proxy#2377) * Feature/add option to skip loading claims from profile url (oauth2-proxy#2329) * add new flag skip-claims-from-profile-url * skip passing profile URL if SkipClaimsFromProfileURL * docs for --skip-claims-from-profile-url flag * update flag comment * update docs * update CHANGELOG.md * Update providers/provider_data.go Co-authored-by: Jan Larwig <jan@larwig.com> * Add tests for SkipClaimsFromProfileURL * simplify tests for SkipClaimsFromProfileURL * generate alpha_config.md --------- Co-authored-by: Jan Larwig <jan@larwig.com> * Add ability to configure username for Redis cluster connections (oauth2-proxy#2381) * Initial attempt. * Add CHANGELOG entry. * Drop commented-out Sentinel test. --------- Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> * chore(deps): update module golang.org/x/crypto to v0.17.0 [security] (oauth2-proxy#2400) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * chore(deps): update github.com/ghodss/yaml digest to d8423dc (oauth2-proxy#2401) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * Improved dev environment (oauth2-proxy#2211) * Improved dev env setup * Cleanup duplicate checks * Applied PR feedback * Updated go.mod/go.sum * go mod tidy * Update .devcontainer/devcontainer.json * Update pkg/http/server_test.go Co-authored-by: Jan Larwig <jan@larwig.com> * Create launch.json * Update .devcontainer/Dockerfile * Apply suggestions from code review --------- Co-authored-by: Jan Larwig <jan@larwig.com> * feature: add release automation workflows (oauth2-proxy#2224) * feature: add release automation workflows * deactivate provenancee because of behaviour change with buildx v0.10.0 * add changelog section extraction for github release notes * fix registry path; fix EOF * use correct version of golangci-lint; add additional workflow step for fetching all dependencies * chore(deps): update module github.com/bsm/redislock to v0.9.4 (oauth2-proxy#2406) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> * initial commit for docusaurus 3 upgrade * fix mdx errors * fix mdx issues * fix routing issues * update docs generation workflow * fix version * fix permissions * move slack to header * remove background color and minify * Update docs/docs/configuration/providers/openid_connect.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/openid_connect.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/openid_connect.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/gitlab.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/gitlab.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/github.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/github.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/github.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> * Update docs/docs/configuration/providers/github.md Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> --------- Co-authored-by: Koen van Zuijlen <8818390+kvanzuijlen@users.noreply.github.com> Co-authored-by: Joel Speed <Joel.speed@hotmail.co.uk> Co-authored-by: Jan Brezina <brezinajn@users.noreply.github.com> Co-authored-by: WhiteRabbit-Code <sven@ertel-net.de> Co-authored-by: Sven Ertel <sven.ertel@bayernwerk.de> Co-authored-by: charvadzo <120425386+charvadzo@users.noreply.github.com> Co-authored-by: Ondrej Charvat <ondrej.charvat@yunextraffic.com> Co-authored-by: Haydn Evans <h.evans@douglas.de> Co-authored-by: Nils Gustav Stråbø <65334626+nilsgstrabo@users.noreply.github.com> Co-authored-by: Ross Golder <ross@golder.org> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Add possibility to encode the state param as UrlEncodedBase64

9d6c86f

brezinajn requested a review from a team as a code owner November 7, 2023 12:23

github-actions bot added docs go tests labels Nov 7, 2023

brezinajn mentioned this pull request Nov 7, 2023

Implement optional Base64Url encoding for the state param #2239

Closed

3 tasks

Update CHANGELOG.md

1c007fd

github-actions bot added the changelog label Nov 7, 2023

tuunit approved these changes Nov 20, 2023

View reviewed changes

tuunit added the LGTM PR is ready to merge label Nov 20, 2023

JoelSpeed previously approved these changes Nov 25, 2023

View reviewed changes

Merge branch 'master' into feature/state-variable-base64-encoding

5913005

brezinajn dismissed JoelSpeed’s stale review via 5913005 November 27, 2023 09:39

JoelSpeed previously approved these changes Dec 3, 2023

View reviewed changes

tuunit requested changes Dec 17, 2023

View reviewed changes

oauthproxy.go Outdated Show resolved Hide resolved

tuunit mentioned this pull request Dec 17, 2023

Feature/multi tenancy #1923

Open

3 tasks

Update oauthproxy.go

29586ff

Co-authored-by: Jan Larwig <jan@larwig.com>

brezinajn dismissed JoelSpeed’s stale review via 29586ff December 19, 2023 13:45

Merge branch 'master' into feature/state-variable-base64-encoding

030314b

kvanzuijlen approved these changes Jan 18, 2024

View reviewed changes

JoelSpeed approved these changes Jan 20, 2024

View reviewed changes

JoelSpeed merged commit bc022fb into oauth2-proxy:master Jan 20, 2024

BrewTestBot mentioned this pull request Feb 13, 2024

oauth2_proxy 7.6.0 Homebrew/homebrew-core#162510

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Add possibility to encode the state param as UrlEncodedBase64 #2312

Add possibility to encode the state param as UrlEncodedBase64 #2312

Uh oh!

brezinajn commented Nov 7, 2023 •

edited

Loading

Uh oh!

JoelSpeed left a comment

Uh oh!

JoelSpeed commented Nov 25, 2023

Uh oh!

brezinajn commented Nov 27, 2023

Uh oh!

tuunit commented Dec 3, 2023

Uh oh!

JoelSpeed commented Dec 3, 2023

Uh oh!

tuunit commented Dec 17, 2023

Uh oh!

Uh oh!

brezinajn commented Dec 19, 2023

Uh oh!

oshmyrko commented Jan 18, 2024

Uh oh!

kvanzuijlen commented Jan 18, 2024

Uh oh!

Uh oh!

Uh oh!

Add possibility to encode the state param as UrlEncodedBase64 #2312

Add possibility to encode the state param as UrlEncodedBase64 #2312

Uh oh!

Conversation

brezinajn commented Nov 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Checklist:

Uh oh!

JoelSpeed left a comment

Choose a reason for hiding this comment

Uh oh!

JoelSpeed commented Nov 25, 2023

Uh oh!

brezinajn commented Nov 27, 2023

Uh oh!

tuunit commented Dec 3, 2023

Uh oh!

JoelSpeed commented Dec 3, 2023

Uh oh!

tuunit commented Dec 17, 2023

Uh oh!

Uh oh!

brezinajn commented Dec 19, 2023

Uh oh!

oshmyrko commented Jan 18, 2024

Uh oh!

kvanzuijlen commented Jan 18, 2024

Uh oh!

Uh oh!

brezinajn commented Nov 7, 2023 •

edited

Loading