Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

stages/email: Fix query parameters getting lost in Email links #5376

Merged
merged 6 commits into from
Oct 19, 2023

Conversation

roney492
Copy link
Contributor

Changes

Bugfix for #5149

New Features

Fixes the bug with the broken email confirmation flow.

Breaking Changes

None

@codecov
Copy link

codecov bot commented Apr 25, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (acad3c4) 92.64% compared to head (c233a58) 92.66%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5376      +/-   ##
==========================================
+ Coverage   92.64%   92.66%   +0.01%     
==========================================
  Files         584      584              
  Lines       28707    28766      +59     
==========================================
+ Hits        26595    26655      +60     
+ Misses       2112     2111       -1     
Flag Coverage Δ
e2e 51.17% <20.00%> (-0.01%) ⬇️
integration 26.07% <3.07%> (-0.06%) ⬇️
unit 89.65% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
authentik/stages/email/stage.py 95.83% <100.00%> (+0.48%) ⬆️
authentik/stages/email/tests/test_stage.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tanberry tanberry requested a review from BeryJu April 26, 2023 13:20
@BeryJu
Copy link
Member

BeryJu commented May 3, 2023

Sorry @roney492 for taking so long to review this, just to clarify, this fixes an issue when using an OIDC-based application x, the user attempts to access application x and then enrolls and account with a flow that has an email stage, they are not redirected correctly to application x after confirming their email, right?

@roney492
Copy link
Contributor Author

roney492 commented May 4, 2023

Hi @BeryJu,
Yes, that's correct!!

@BeryJu
Copy link
Member

BeryJu commented May 10, 2023

@roney492 I've done some testing with and without this PR and both worked correctly to keep the redirect URL. Could you post the Flow you're using for this?

@roney492
Copy link
Contributor Author

Sure,
enrollment_email.zip

@BeryJu BeryJu requested a review from a team as a code owner October 19, 2023 14:56
@netlify
Copy link

netlify bot commented Oct 19, 2023

Deploy Preview for authentik-storybook canceled.

Name Link
🔨 Latest commit c233a58
🔍 Latest deploy log https://app.netlify.com/sites/authentik-storybook/deploys/65314d208368810009a6b8ca

Signed-off-by: Jens Langhammer <jens@goauthentik.io>
@BeryJu BeryJu changed the title Email confirmation flow fix for Application based flows stages/email: Fix query parameters getting lost in Email links Oct 19, 2023
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
@BeryJu BeryJu requested a review from a team as a code owner October 19, 2023 15:32
Signed-off-by: Jens Langhammer <jens@goauthentik.io>
@BeryJu BeryJu merged commit f036820 into goauthentik:main Oct 19, 2023
63 checks passed
kensternberg-authentik added a commit that referenced this pull request Oct 19, 2023
* main: (57 commits)
  stages/email: Fix query parameters getting lost in Email links (#5376)
  core/rbac: fix missing field when removing perm, add delete from object page (#7226)
  website/integrations: grafana: add Helm and Terraform config examples (#7121)
  web: bump @types/codemirror from 5.60.11 to 5.60.12 in /web (#7223)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7224)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7225)
  website/blogs: blog about sso tax (#7202)
  web: Application wizard v2 with tests (#7004)
  web: bump API Client version (#7220)
  core: bump goauthentik.io/api/v3 from 3.2023083.7 to 3.2023083.8 (#7221)
  providers/radius: TOTP MFA support (#7217)
  web: bump API Client version (#7218)
  stage/deny: add custom message (#7144)
  docs: update full-dev-setup docs (#7205)
  enterprise: bump license usage task frequency (#7215)
  web: bump the storybook group in /web with 5 updates (#7212)
  web: bump the sentry group in /web with 2 updates (#7211)
  Revert "web: Updates to the Context and Tasks libraries from lit. (#7168)"
  web: bump @types/codemirror from 5.60.10 to 5.60.11 in /web (#7209)
  web: bump @types/chart.js from 2.9.38 to 2.9.39 in /web (#7206)
  ...
kensternberg-authentik added a commit that referenced this pull request Oct 19, 2023
* main: (119 commits)
  stages/email: Fix query parameters getting lost in Email links (#5376)
  core/rbac: fix missing field when removing perm, add delete from object page (#7226)
  website/integrations: grafana: add Helm and Terraform config examples (#7121)
  web: bump @types/codemirror from 5.60.11 to 5.60.12 in /web (#7223)
  translate: Updates for file web/xliff/en.xlf in zh_CN (#7224)
  translate: Updates for file web/xliff/en.xlf in zh-Hans (#7225)
  website/blogs: blog about sso tax (#7202)
  web: Application wizard v2 with tests (#7004)
  web: bump API Client version (#7220)
  core: bump goauthentik.io/api/v3 from 3.2023083.7 to 3.2023083.8 (#7221)
  providers/radius: TOTP MFA support (#7217)
  web: bump API Client version (#7218)
  stage/deny: add custom message (#7144)
  docs: update full-dev-setup docs (#7205)
  enterprise: bump license usage task frequency (#7215)
  web: bump the storybook group in /web with 5 updates (#7212)
  web: bump the sentry group in /web with 2 updates (#7211)
  Revert "web: Updates to the Context and Tasks libraries from lit. (#7168)"
  web: bump @types/codemirror from 5.60.10 to 5.60.11 in /web (#7209)
  web: bump @types/chart.js from 2.9.38 to 2.9.39 in /web (#7206)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants