-
Notifications
You must be signed in to change notification settings - Fork 118
0.7 Release #139
Changes from all commits
c8475e3
79f24db
d3655f0
f7fa8cc
b3bd9a2
72891cc
2e35da0
36cc00f
6736a07
a0b2ed4
8405986
0dc0ede
44aa4c3
72be2f8
2363314
c73fea5
1905e0d
1df214c
633775a
8148f8e
e80a1ba
944d8d2
d17adbf
24c67e0
b92eace
ab53e19
0c4f9cd
01a4a6b
96ab0d1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,13 +21,19 @@ const PROVIDERS = { | |
// Options that apply to any provider | ||
const defaults = { | ||
idField: '_id', | ||
shouldSetupSuccessRoute: true, | ||
shouldSetupFailureRoute: true, | ||
successRedirect: '/auth/success', | ||
failureRedirect: '/auth/failure', | ||
tokenEndpoint: '/auth/token', | ||
localEndpoint: '/auth/local', | ||
userEndpoint: '/users', | ||
header: 'authorization', | ||
cookie: 'feathers-jwt' | ||
cookie: { | ||
name: 'feathers-jwt', | ||
httpOnly: false, | ||
secure: process.env.NODE_ENV === 'production' ? true : false | ||
} | ||
}; | ||
|
||
export default function auth(config = {}) { | ||
|
@@ -52,27 +58,17 @@ export default function auth(config = {}) { | |
|
||
// Merge and flatten options | ||
const authOptions = Object.assign({}, defaults, app.get('auth'), config); | ||
|
||
// If we should redirect on success and the redirect route is the same as the | ||
// default then we'll set up a route handler. Otherwise we'll leave it to the developer | ||
// to set up their own custom route handler. | ||
if (authOptions.successRedirect === defaults.successRedirect) { | ||
debug(`Setting up successRedirect route: ${authOptions.successRedirect}`); | ||
|
||
app.get(authOptions.successRedirect, function(req, res){ | ||
res.sendFile(path.resolve(__dirname, 'public', 'auth-success.html')); | ||
}); | ||
|
||
// If a custom success redirect is passed in or it is disabled then we | ||
// won't setup the default route handler. | ||
if (authOptions.successRedirect !== defaults.successRedirect) { | ||
authOptions.shouldSetupSuccessRoute = false; | ||
} | ||
|
||
// If we should redirect on failure and the redirect route is the same as the | ||
// default then we'll set up a route handler. Otherwise we'll leave it to the developer | ||
// to set up their own custom route handler. | ||
if (authOptions.failureRedirect === defaults.failureRedirect) { | ||
debug(`Setting up failureRedirect route: ${authOptions.failureRedirect}`); | ||
|
||
app.get(authOptions.failureRedirect, function(req, res){ | ||
res.sendFile(path.resolve(__dirname, 'public', 'auth-fail.html')); | ||
}); | ||
// If a custom failure redirect is passed in or it is disabled then we | ||
// won't setup the default route handler. | ||
if (authOptions.failureRedirect !== defaults.failureRedirect) { | ||
authOptions.shouldSetupFailureRoute = false; | ||
} | ||
|
||
// Set the options on the app | ||
|
@@ -115,7 +111,7 @@ export default function auth(config = {}) { | |
// be dealing with a config param and not a provider config | ||
// If that's the case we don't need to merge params and we | ||
// shouldn't try to set up a service for this key. | ||
if (!isObject(config[key])) { | ||
if (!isObject(config[key]) || key === 'cookie') { | ||
return; | ||
} | ||
|
||
|
@@ -145,6 +141,24 @@ export default function auth(config = {}) { | |
// Register error handling middleware for redirecting to support | ||
// redirecting on authentication failure. | ||
app.use(middleware.failedLogin(authOptions)); | ||
|
||
// Setup route handler for default success redirect | ||
if (authOptions.shouldSetupSuccessRoute) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What was the reason again for adding those? I mean if you do a redirect it will almost certainly be to your own success page no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is if someone wants to use the same route name but a different handler. Which, I think is actually probably the most common use case. You can't rely on just inspecting whether the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What I'm saying is why are there default auth redirect HTML pages at all? I might be missing something but they don't seem particularly useful. If you do a browser redirect it'll pretty much always be to your app right? Couldn't we make it required and throw an error if it isn't set (or disabled)? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For web apps you are correct, you would pretty much always override them. However, if you use the pop-up method or are on mobile, with the defaults you don't need to do anything to the server. You check that you navigated to |
||
debug(`Setting up successRedirect route: ${authOptions.successRedirect}`); | ||
|
||
app.get(authOptions.successRedirect, function(req, res){ | ||
res.sendFile(path.resolve(__dirname, 'public', 'auth-success.html')); | ||
}); | ||
} | ||
|
||
// Setup route handler for default failure redirect | ||
if (authOptions.shouldSetupFailureRoute) { | ||
debug(`Setting up failureRedirect route: ${authOptions.failureRedirect}`); | ||
|
||
app.get(authOptions.failureRedirect, function(req, res){ | ||
res.sendFile(path.resolve(__dirname, 'public', 'auth-fail.html')); | ||
}); | ||
} | ||
}; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a standard default?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It just needs to be set in the past, so it can be whatever.