Skip to content
This repository has been archived by the owner on Apr 14, 2021. It is now read-only.

Check authorised user is the expected user #10

Merged
merged 4 commits into from
Aug 10, 2017
Merged

Check authorised user is the expected user #10

merged 4 commits into from
Aug 10, 2017

Conversation

andyhd
Copy link
Contributor

@andyhd andyhd commented Aug 10, 2017

Users were able to login to other users' RStudio instances.
This prevents that.
Users can still login to their own RStudio instance, but get a 403 "Forbidden" error if they login to another user's RStudio.

@andyhd andyhd requested review from kerin and xoen August 10, 2017 11:14
routes/index.js Outdated
function authorisedUser(req) {

if (req && req.user && req.user.nickname) {
return req.user.nickname.toLowerCase() == env.USER;
Copy link
Contributor

Choose a reason for hiding this comment

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

As this is JS (😱), I would use === here to be on the safe side.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

== coerces the type of the operands, which can be a problem when comparing strings and integers, for example, but it's not a problem here. But I will change this for best practices sake.

@xoen
Copy link
Contributor

xoen commented Aug 10, 2017

See comment on equality but as far as I can see looks good.

@andyhd andyhd merged commit 6c05402 into master Aug 10, 2017
@andyhd andyhd deleted the fix-auth branch August 10, 2017 12:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants