spring-attic / spring-social

Allows you to connect your applications with SaaS providers such as Facebook and Twitter.
http://projects.spring.io/spring-social
Apache License 2.0
619 stars 351 forks source link

Fix OAuth1 redirect URL when server is behind a proxy. #261

Closed brendanjbaker closed 5 years ago

brendanjbaker commented 6 years ago

This is the same fix for OAuth1AuthenticationService that was applied to OAuth2AuthenticationService in commit 385e7ca (SOCIAL-447). Specifically, if the Host header is present in the request, the X-Forwarded-Proto and X-Forwarded-Port headers are used to generate the callback URL. This is because a reverse proxy may receive a request via HTTPS on port 443, but an internal application server will see the request as coming via HTTP over port 8080 (for example), and that information will subsequently be used to generate an incorrect redirect URL.

The callback URL automatically generated by OAuth1AuthenticationService was:

http://example.com/auth/twitter

...Despite our site being HTTPS. Due to Twitter recently beginning to enforce strict callback URL matching, Twitter logins were broken because our Twitter application configuration defined our callback URL as:

https://example.com/auth/twitter

I am using this fork in my production application and Twitter logins are now working. My specific setup is Spring Boot 2.0.5 running on Jetty, hosted behind nginx. The site is HTTPS-only, and HTTPS connections are terminated at nginx and forwarded to our internal Jetty host over HTTP, which is running on port 8080.

In case it's helpful for anybody, my nginx configuration for the reverse proxy section is as follows:

proxy_set_header Host $host;
proxy_set_header X-Forwarded-For $proxy_add_x_forwarded_for;
proxy_set_header X-Forwarded-Port $server_port;
proxy_set_header X-Forwarded-Proto $scheme;
proxy_set_header X-Real-IP $remote_addr;

Tagging @jaffadog because he was the author of the original OAuth2AuthenticationService fix, in case he has any interest in following this or providing any feedback.

Tagging @codeconsole because he has an open pull request (#259) that alters the corresponding logic in OAuth2AuthenticationService. You may want to include the same change in OAuth1AuthenticationService if/when this is merged.

pivotal-issuemaster commented 6 years ago

@brendanjbaker Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

pivotal-issuemaster commented 6 years ago

@brendanjbaker Thank you for signing the Contributor License Agreement!

codeconsole commented 6 years ago

Thanks for tagging me, this really seems like it should be moved into AbstractSocialAuthenticationService.java to prevent code duplication (especially considering the original solution doesn't work in all environments). #259 now includes your fix.

brendanjbaker commented 5 years ago

@codeconsole I agree fully. Since #259 has the same effect (and fix) as this, and since the other change included in #259 seems quite agreeable, I'm going to close this pull request.

@habuma -- Can you please review and/or approve/merge #259? Than you!