Skip to content

Commit

Permalink
Deprecate some routes (#1199)
Browse files Browse the repository at this point in the history
* Deprecate some routes

* Normalize on `/login`... keep `register` to ourselves in the code... we already do this a lot... think `index.js` vs `main`.
* Removed one dead route... why would we want to do `/github` to `/`... means to me that it's dead... just return a default 404.
* Fixed one doc send with status 302 to redirect instead... default is 302 ... would use a 307 but that's only HTTP/1.1 and would definitely rule out any older browser although may be reconsidered at a later date
* Some white space treatment

Applies to #1198 #135 and post #1174

* Undeprecate a route

* Added a note

Applies to #135

* Remove this route since it won't exist

Applies to #135

* Revert last commit

* Will exist for hook but root of it won't

Applies to #135
  • Loading branch information
Martii authored Oct 24, 2017
1 parent b54f20a commit 5c85bfa
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 17 deletions.
13 changes: 6 additions & 7 deletions controllers/auth.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ exports.auth = function (aReq, aRes, aNext) {
var strategy = aReq.body.auth || aReq.params.strategy;
var username = aReq.body.username || aReq.session.username ||
(authedUser ? authedUser.name : null);
var authOpts = { failureRedirect: '/register?stratfail' };
var authOpts = { failureRedirect: '/login?stratfail' };
var passportKey = aReq._passport.instance._key;

// Yet another passport hack.
Expand All @@ -118,7 +118,7 @@ exports.auth = function (aReq, aRes, aNext) {
}

if (!username) {
aRes.redirect('/register?noname');
aRes.redirect('/login?noname');
return;
}
// Clean the username of leading and trailing whitespace,
Expand All @@ -127,7 +127,7 @@ exports.auth = function (aReq, aRes, aNext) {

// The username could be empty after the replacements
if (!username) {
aRes.redirect('/register?noname');
aRes.redirect('/login?noname');
return;
}

Expand Down Expand Up @@ -164,7 +164,7 @@ exports.auth = function (aReq, aRes, aNext) {
}

if (!strategy) {
aRes.redirect('/register');
aRes.redirect('/login');
return;
} else {
auth();
Expand Down Expand Up @@ -243,7 +243,7 @@ exports.callback = function (aReq, aRes, aNext) {
console.error(colors.red('`User` not found'));
}

aRes.redirect(doneUri + (doneUri === '/' ? 'register' : '') + '?authfail');
aRes.redirect(doneUri + (doneUri === '/' ? 'login' : '') + '?authfail');
return;
}

Expand Down Expand Up @@ -296,8 +296,7 @@ exports.callback = function (aReq, aRes, aNext) {

exports.validateUser = function validateUser(aReq, aRes, aNext) {
if (!aReq.session.user) {
aRes.location('/login');
aRes.status(302).send();
aRes.redirect('/login');
return;
}
aNext();
Expand Down
2 changes: 1 addition & 1 deletion controllers/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ exports.register = function (aReq, aRes) {
options.redirectTo = getRedirect(aReq);

// Page metadata
pageMetadata(options, 'Register');
pageMetadata(options, 'Login');

// Session
options.authedUser = authedUser = modelParser.parseUser(authedUser);
Expand Down
16 changes: 11 additions & 5 deletions routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,9 @@ module.exports = function (aApp) {
aApp.route('/auth/:strategy').get(authentication.auth);
aApp.route('/auth/:strategy/callback/:junk?').get(authentication.callback);
aApp.route('/login').get(main.register);
aApp.route('/register').get(main.register);
aApp.route('/register').get(function (aReq, aRes) {
aRes.redirect(301, '/login');
});
aApp.route('/logout').get(main.logout);

// User routes
Expand All @@ -44,8 +46,11 @@ module.exports = function (aApp) {
aApp.route('/users/:username/github/import').post(authentication.validateUser, user.userGitHubImportScriptPage);
aApp.route('/users/:username/profile/edit').get(authentication.validateUser, user.userEditProfilePage).post(authentication.validateUser, user.update);
aApp.route('/users/:username/update').post(admin.adminUserUpdate);
// NOTE: Some below inconsistent with priors
aApp.route('/user/preferences').get(authentication.validateUser, user.userEditPreferencesPage);
aApp.route('/user').get(function (aReq, aRes) { aRes.redirect('/users'); });
aApp.route('/user').get(function (aReq, aRes) {
aRes.redirect(302, '/users');
});
aApp.route('/api/user/exist/:username').head(user.exist);

// Adding script/library routes
Expand All @@ -55,14 +60,16 @@ module.exports = function (aApp) {
aApp.route('/user/add/lib').get(authentication.validateUser, user.newLibraryPage);
aApp.route('/user/add/lib/new').get(script.new(script.lib(user.editScript))).post(authentication.validateUser, script.new(script.lib(user.submitSource)));
aApp.route('/user/add/lib/upload').post(authentication.validateUser, script.lib(user.uploadScript));
aApp.route('/user/add').get(function (aReq, aRes) { aRes.redirect('/user/add/scripts'); });
aApp.route('/user/add').get(function (aReq, aRes) {
aRes.redirect(301, '/user/add/scripts');
});

// Script routes
aApp.route('/scripts/:username/:scriptname').get(script.view);
aApp.route('/scripts/:username/:scriptname/edit').get(authentication.validateUser, script.edit).post(authentication.validateUser, script.edit);
aApp.route('/scripts/:username/:scriptname/source').get(user.editScript);
aApp.route('/scripts/:username').get(function (aReq, aRes) {
aRes.redirect('/users/' + aReq.params.username + '/scripts'); // NOTE: Watchpoint
aRes.redirect(301, '/users/' + aReq.params.username + '/scripts'); // NOTE: Watchpoint
});

aApp.route('/install/:username/:scriptname').get(scriptStorage.unlockScript, scriptStorage.sendScript);
Expand All @@ -72,7 +79,6 @@ module.exports = function (aApp) {
// Github hook routes
aApp.route('/github/hook').post(scriptStorage.webhook);
aApp.route('/github/service').post(function (aReq, aRes, aNext) { aNext(); });
aApp.route('/github').get(function (aReq, aRes) { aRes.redirect('/'); });

// Library routes
aApp.route('/libs/:username/:scriptname').get(script.lib(script.view));
Expand Down
2 changes: 1 addition & 1 deletion views/includes/header.html
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
<li><a href="/logout" title="Sign Out"><span class="visible-xs-inline">Sign Out</span><i class="fa fa-sign-out"></i></a></li>
{{/authedUser}}
{{^authedUser}}
<li><a href="/register">Sign In / Sign Up <i class="fa fa-sign-in"></i></a></li>
<li><a href="/login">Sign In / Sign Up <i class="fa fa-sign-in"></i></a></li>
{{/authedUser}}
</ul>
</div>
Expand Down
6 changes: 3 additions & 3 deletions views/pages/loginPage.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
<form action="/auth/" method="post" class="form-register">
<input type="hidden" name="redirectTo" value="{{{redirectTo}}" />
<h3>
Register
Login
<span class="help-block small">Enter your username for our site to use with your preferred <a href="https://www.wikipedia.org/wiki/OAuth">OAuth</a>/<a href="https://www.wikipedia.org/wiki/OpenID">OpenID</a> service.</span>
</h3>
<noscript>
<div class="alert alert-danger small" role="alert">
<i class="fa fa-exclamation-triangle"></i> <strong>WARNING</strong>: The username entered may include additional sanitizing for web friendly URLs and Userscript engine compatibility. If you wish to see the sanitization changes please enable JavaScript and <a href="/register">start over</a>.
<i class="fa fa-exclamation-triangle"></i> <strong>WARNING</strong>: The username entered may include additional sanitizing for web friendly URLs and Userscript engine compatibility. If you wish to see the sanitization changes please enable JavaScript and <a href="/login">start over</a>.
</div>
</noscript>
<div class="input-group">
Expand All @@ -32,7 +32,7 @@ <h3>
</span>
</div>
<div class="alert alert-warning small" role="alert">
<i class="fa fa-exclamation-triangle"></i> <strong>CAUTION</strong>: The username that you choose to register will be displayed to everyone. It is strongly recommended to <strong>not</strong> attempt to use an email address.
<i class="fa fa-exclamation-triangle"></i> <strong>CAUTION</strong>: The username that you choose to login with will be displayed to everyone. It is strongly recommended to <strong>not</strong> attempt to use an email address.
</div>
<ul class="nav nav-pills nav-justified">
<li><a href="/about/Privacy-Policy"><i class="fa fa-user-secret"></i> Privacy Policy</a></li>
Expand Down

1 comment on commit 5c85bfa

@Martii
Copy link
Member Author

@Martii Martii commented on 5c85bfa Oct 24, 2017

Choose a reason for hiding this comment

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

Cc: @cvzi

We are breaking one of your userscripts with this... please change register search to login.

Also there is an issue with your account that I am going to twiddle with... you seem to have two GH auths... I'm going to kill the older one. If you have any issues logging in I'll restore the older one and get rid of the newer one. Please give me a mention here if you are having issues with logging in after I clean it up.

Thanks.


Done

Please sign in to comment.