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

fix(core): Redirect to login page rather than 403 if not signed in #1496

Merged
merged 1 commit into from
Sep 13, 2016

Conversation

hyperreality
Copy link
Contributor

feat(users): Redirect to login page rather than 403 if not signed in

Non-signed in users are being redirected to the unfriendly 403 forbidden page instead of the sign in page as should be the case. Fixed the tiny bug in the code that caused this to happen.

@coveralls
Copy link

coveralls commented Sep 9, 2016

Coverage Status

Coverage remained the same at 72.998% when pulling 319d21b on hyperreality:statechange into 17772fe on meanjs:master.

@mleanos
Copy link
Member

mleanos commented Sep 10, 2016

@hyperreality Aren't there instances where we would have an undefined User in the Authentication service?

Maybe we should check for both null, and undefined?

Or maybe simpler to do:

if (!Authentication.user) { 
  // redirect to login 
} else {
  // redirect to forbidden 
}

@hyperreality
Copy link
Contributor Author

I considered that, but after playing around with the app a bit I couldn't reach a state where Authentication.user was undefined. var user = null is embedded in the layout. I liked specifying null directly because it is more explicit.

I'm not sure we need to worry about the case where people will create a different layout view in which the user object isn't embedded, because then they would have to rewrite the entire stateChangeStart function anyway. Thoughts?

@mleanos
Copy link
Member

mleanos commented Sep 10, 2016

Perfect answer! Thanks! LGTM.

We'll wait to see if others want to chime in over the next day or so.

@mleanos mleanos added this to the 0.5.0 milestone Sep 10, 2016
@mleanos mleanos self-assigned this Sep 10, 2016
@simison
Copy link
Member

simison commented Sep 10, 2016

BTW this is good to keep in mind with JS:

foo === null

Matches only null as it should.

but...

foo == null

equals:

foo === null || foo === undefined

@hyperreality
Copy link
Contributor Author

hyperreality commented Sep 10, 2016

@simison, yes, sometimes that shortcut is helpful, but eslint will complain about the lazy comparison operator

@mleanos mleanos merged commit 8b54669 into meanjs:master Sep 13, 2016
@frkwhiteangel
Copy link

I dont get this to work? its still not redirecting for me.

hyperreality added a commit to hyperreality/mean that referenced this pull request Sep 17, 2016
Wuntenn pushed a commit to Wuntenn/mean that referenced this pull request Sep 20, 2016
mleanos pushed a commit that referenced this pull request Oct 5, 2016
* Added configuration for owasp. Synchronize client owap configs with the server configs.
Also added a time indicator on failed login attempts to give the user feedback on subsequent failed login attempts.

* switched to handlebar template for passing the server's owasp config down to the client.

reverted some of the other changes (regarding the http request).

* Removed debug code.

* Changed variable name to owaspConfig

* Fixed minor type-o's and set owasp.config() rather than the underlying configs.

* chore(tidy): tidying up minor lint and layout issues

* fix(lint): CSS alphabetize warnings (#1498)

Fixes css lintings warnings of properties not alphabetized.

* fix(authentication) Stops error on signin/signup (#1495)

Uses the passport info object to simplify login and remove the need to
temporarily cache the redirect within the session.

* Moved owasp config into default and reverted other config files.

Modified config to be "shared". This will allow future configurations to be easily passed to the client.

* fixed 403 redirect if not signed in (#1496)

* Update form-article.client.view.html

For New Article, delete function no required

* UI changes for mobile; autofocus

* fixed broken password popover balloon

* add e2e test for autofocus

* Remove test, fix delete social login button

* feat(core): Move template to .github folder

* Deprecated $http success/error promise methods (#1508)

Replaces the $http service calls with promise based methods
of the client-side UsersService for the following:
  Users Change Password
  Users Manage Social Accounts
  Users Password Forgot
  Users Password Reset
  Users Signup
  Users Signin

Modifies tests to reflect changes.

Closes #1479

* rebase
@hyperreality hyperreality deleted the statechange branch November 20, 2016 02:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants