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

Make forceHttps complete #60

Merged
merged 3 commits into from Feb 24, 2015
Merged

Make forceHttps complete #60

merged 3 commits into from Feb 24, 2015

Conversation

@dapus
Copy link
Contributor

dapus commented Feb 21, 2015

redirect_uri is sent to the provider a second time. This code forces https (if option is set) in that case as well.

@@ -176,12 +176,17 @@ exports.v2 = function (settings) {
return reply(Boom.internal('Incorrect ' + name + ' state parameter'));
}

var protocol = request.connection.info.protocol;
if (settings.forceHttps) {
protocol = 'https';

This comment has been minimized.

Copy link
@geek

geek Feb 23, 2015

Member

we are somewhat in the minority on this, but we use 4 spaces.

@geek geek self-assigned this Feb 23, 2015
var cookie = res.headers['set-cookie'][0].split(';')[0] + ';';

mock.server.inject(res.headers.location, function (res) {
expect(res.headers.location).to.contain('https://localhost:80/login?code=1&state=');

This comment has been minimized.

Copy link
@geek

geek Feb 23, 2015

Member

after function declarations we expect a newline: http://hapijs.com/styleguide

@dapus

This comment has been minimized.

Copy link
Contributor Author

dapus commented Feb 24, 2015

Thanks for the feedback. This is according to the styleguide, right? 🚀

@geek

This comment has been minimized.

Copy link
Member

geek commented Feb 24, 2015

@dapus ya, the hapijs style guide.

@geek geek added the bug label Feb 24, 2015
@geek geek added this to the 2.2.0 milestone Feb 24, 2015
geek added a commit that referenced this pull request Feb 24, 2015
Make forceHttps complete
@geek geek merged commit a25ee54 into hapijs:master Feb 24, 2015
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.