Skip to content

Added support to set SOCIAL_AUTH_ALLOWED_REDIRECT_HOSTS #429

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

Merged
merged 4 commits into from
Feb 2, 2024

Conversation

rhysyngsun
Copy link
Contributor

@rhysyngsun rhysyngsun commented Jan 30, 2024

What are the relevant tickets?

Part of #420

Description (What does it do?)

Adds a setting for SOCIAL_AUTH_ALLOWED_REDIRECT_HOSTS to support allowing a redirect to OCW on login.

How can this be tested?

  • Navigate to /login, it should forward you on to /login/ol-oidc and when you complete login you end up at the home page.
  • Next try /login?next=http://ocw.odl.local, you should get redirected to /login/ol-oidc/?next=http://ocw.odl.local and then it should ignore the next parameter and redirect you to the home page.
  • In your .env file, set SOCIAL_AUTH_ALLOWED_REDIRECT_HOSTS=["ocw.odl.local"]
  • Now try /login?next=http://ocw.odl.local again, you should get redirected to /login/ol-oidc/?next=http://ocw.odl.local and then subsequently http://ocw.odl.local (doesn't matter that this doesn't exist).

@rhysyngsun rhysyngsun added the Needs Review An open Pull Request that is ready for review label Jan 30, 2024
Copy link

OpenAPI Changes

Show/hide No detectable change.

Copy link

codecov bot commented Jan 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (4550651) 76.85% compared to head (d6233b3) 76.83%.
Report is 9 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #429      +/-   ##
==========================================
- Coverage   76.85%   76.83%   -0.02%     
==========================================
  Files         244      244              
  Lines        9975     9976       +1     
  Branches     1703     1703              
==========================================
- Hits         7666     7665       -1     
- Misses       2141     2142       +1     
- Partials      168      169       +1     

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

@rhysyngsun rhysyngsun force-pushed the nl/ocw-integration-settings branch from 9054d09 to d6233b3 Compare January 30, 2024 20:48
@gumaerc gumaerc self-assigned this Jan 31, 2024
@collinpreston
Copy link
Contributor

Just wanted to check, but has this been tested to verify that the functionality added in #330 will still work?

@rhysyngsun
Copy link
Contributor Author

@collinpreston yeah that'll still work, that code path wasn't touched

Copy link
Contributor

@gumaerc gumaerc left a comment

Choose a reason for hiding this comment

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

👍 LGTM!

I had some unrelated issues testing this that I detailed in this bug report: #445

After figuring that out and correcting the API URL in mit-open-login-button, I was able to append ?redirect_url=http://ocw.odl.local:3000/ to the login URL and everything worked great. I clicked login, followed the login process and was successfully redirected back to my local OCW page.

@gumaerc gumaerc added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Feb 2, 2024
@gumaerc gumaerc assigned rhysyngsun and unassigned gumaerc Feb 2, 2024
@rhysyngsun rhysyngsun merged commit 3043814 into main Feb 2, 2024
@rhysyngsun rhysyngsun deleted the nl/ocw-integration-settings branch February 2, 2024 18:26
@odlbot odlbot mentioned this pull request Feb 7, 2024
26 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants