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.

Sign up for GitHub

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

Jump to bottom

No open redirects for enterprise SSO's #22622

Merged
merged 20 commits into from
May 13, 2022
Merged

No open redirects for enterprise SSO's #22622

merged 20 commits into from
May 13, 2022

Conversation

howonlee
Copy link
Contributor

@howonlee howonlee commented May 11, 2022 •
edited by flamber
Loading

@howonlee howonlee added the backport Automatically create PR on current release branch on merge label May 11, 2022
@codecov
Copy link

Codecov bot commented May 11, 2022 •
edited
Loading

Codecov Report

Merging #22622 (b66ae45) into master (8f5e9f9) will decrease coverage by 0.05%.
The diff coverage is 91.80%.

@@            Coverage Diff             @@
##           master   #22622      +/-   ##
==========================================
- Coverage   64.19%   64.13%   -0.06%     
==========================================
  Files        2446     2456      +10     
  Lines       81119    81268     +149     
  Branches     9914     9933      +19     
==========================================
+ Hits        52075    52125      +50     
- Misses      24945    25044      +99     
  Partials     4099     4099
Flag Coverage Δ
back-end 85.49% <91.80%> (+0.01%) ⬆️
front-end 44.01% <ø> (-0.07%) ⬇️

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

Impacted Files Coverage Δ
...metabase_enterprise/sso/integrations/sso_utils.clj 88.88% <87.50%> (-0.59%) ⬇️
.../src/metabase_enterprise/sso/integrations/saml.clj 75.65% <90.90%> (+0.43%) ⬆️
...d/src/metabase_enterprise/sso/integrations/jwt.clj 93.75% <93.75%> (+0.09%) ⬆️
...se/backend/src/metabase_enterprise/sso/api/sso.clj 100.00% <100.00%> (ø)
frontend/src/metabase-lib/lib/utils.ts 79.54% <0.00%> (-5.31%) ⬇️
frontend/src/metabase/lib/browser.js 60.00% <0.00%> (-1.54%) ⬇️
.../src/metabase-enterprise/group_managers/actions.js 20.68% <0.00%> (-1.54%) ⬇️
...ase/admin/databases/containers/DatabaseEditApp.jsx 83.33% <0.00%> (-1.29%) ⬇️
...end/src/metabase/query_builder/actions/querying.js 23.40% <0.00%> (-0.78%) ⬇️
frontend/src/metabase/dashboard/selectors.js 62.02% <0.00%> (-0.65%) ⬇️
... and 201 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8f5e9f9...b66ae45. Read the comment docs.

@howonlee howonlee marked this pull request as ready for review May 12, 2022 10:50
@howonlee howonlee requested review from bshepherdson and camsaul and removed request for bshepherdson May 12, 2022 20:24
camsaul
camsaul approved these changes May 12, 2022
View reviewed changes
Copy link
Member

@camsaul camsaul left a comment

Choose a reason for hiding this comment

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

Cool this looks good to me.

@howonlee howonlee merged commit f3f9c62 into master May 13, 2022
@howonlee howonlee deleted the boat160-sso branch May 13, 2022 07:04
@github-actions
Copy link
Contributor

@howonlee could not create a backport due to conflicts

@rlotun
Copy link
Contributor

rlotun commented May 13, 2022

we need to manually backport this @howonlee

howonlee added a commit that referenced this pull request May 16, 2022
@howonlee
No open redirects for enterprise SSO's (#22622)
2d76adb 
Open redirects means doing some sso with a built-in redirect, and redirecting into an unhappy place (aka, a non-MB place) afterwards so that someone gets phished or other bad things happen. This is already prevented for OSS sso's but not EE - prevents this for EE sso's by forcing redirects to be in MB `site-url` set domain.
rlotun pushed a commit that referenced this pull request May 16, 2022
@howonlee
Manual backport for No open redirects for enterprise SSO's (#22622) (…
308f652 
…#22745)

* No open redirects for enterprise SSO's (#22622)

Open redirects means doing some sso with a built-in redirect, and redirecting into an unhappy place (aka, a non-MB place) afterwards so that someone gets phished or other bad things happen. This is already prevented for OSS sso's but not EE - prevents this for EE sso's by forcing redirects to be in MB `site-url` set domain.

* lint fix
cure53
cure53 reviewed Jun 11, 2022
View reviewed changes
Copy link

@cure53 cure53 left a comment

Choose a reason for hiding this comment

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

This looks good ✔️

@noahmoss noahmoss mentioned this pull request Aug 1, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cure53 cure53 cure53 left review comments

@camsaul camsaul camsaul approved these changes

Assignees
No one assigned
Labels
backport Automatically create PR on current release branch on merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@howonlee @rlotun @camsaul @cure53