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

Support for urlSSL config option and forceAdminSSL 403 response #2238

Merged
merged 1 commit into from
Apr 28, 2014

Conversation

gimelfarb
Copy link
Contributor

closes #1838

  • adding forceAdminSSL: {redirect: true/false} option to allow 403 over non-SSL rather than redirect
  • adding urlSSL option to specify SSL variant of url
  • using urlSSL when redirecting to SSL (forceAdminSSL), if specified
  • dynamically patching .url property for view engine templates to use SSL variant over HTTPS connections (pass .secure property as view engine data)
  • using urlSSL in a "reset password" email, if specified
  • adding unit tests to test forceAdminSSL and urlSSL options
  • created a unit test utility function to dynamically fork a new instance of Ghost during the test, with different configuration options

@sebgie
Copy link
Contributor

sebgie commented Mar 27, 2014

I'm very sorry for taking so long to come back to your PR. I have tested the functionality and it is working as expected 👍. Due to the change from jslint to jshint the tests are failing and the PR would need rebasing (only a minor conflict).

If you have a moment could you eventually update the PR or leave a comment if you have no time to do the changes? Thank you!

@gimelfarb
Copy link
Contributor Author

Yeah ok I can update it. Might be tight this week though, and then away for another week. If I don't do it this week, I'll do it when I come back. Will need to re-check what has changed in the codebase since then with regards to .url handling, and if any new bits would need updating.

@ErisDS
Copy link
Member

ErisDS commented Apr 3, 2014

@gimelfarb excellent, just drop us a comment let us know when you're done. 0.5 doesn't close for some time yet but if at any point it becomes clear you don't have time, just drop us a comment to let us know that instead 👍

closes TryGhost#1838
- adding `forceAdminSSL: {redirect: true/false}` option to allow 403 over non-SSL rather than redirect
- adding `urlSSL` option to specify SSL variant of `url`
- using `urlSSL` when redirecting to SSL (forceAdminSSL), if specified
- dynamically patching `.url` property for view engine templates to use SSL variant over HTTPS connections (pass `.secure` property as view engine data)
- using `urlSSL` in a "reset password" email, if specified
- adding unit tests to test `forceAdminSSL` and `urlSSL` options
- created a unit test utility function to dynamically fork a new instance of Ghost during the test, with different configuration options
@gimelfarb
Copy link
Contributor Author

Ok finally got around to doing this. Rebased the commit, fixed merging issues, and made sure the tests pass and build is all "green". All yours to merge! :)

ErisDS added a commit that referenced this pull request Apr 28, 2014
Support for urlSSL config option and forceAdminSSL 403 response
@ErisDS ErisDS merged commit 3ffa552 into TryGhost:master Apr 28, 2014
ErisDS added a commit that referenced this pull request Apr 28, 2014
fixes the build

- PR #2238 added an extra reference to aspect which wasn't fixed by #2652, this resolves that
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.

forceAdminSSL may need to redirect to another domain
3 participants