search

saxifrage / cityasacampus

An open-source platform for connecting and showcasing resources within local learning communities.
http://cityasacampus.org/
5 stars 4 forks source link

implement token auth #357

Closed chadwhitacre closed 8 years ago

chadwhitacre commented 8 years ago

This is for #347, ya @dmtroyer @MatthewVita?

chadwhitacre commented 8 years ago

The three provisioning commits (3420524, 09bb778, 3a289d9) seem out of place on this PR, and are the source of the conflicts with master (cf. #355 #356). I'm going to rebase them off of this branch.

chadwhitacre commented 8 years ago

Done. Previous head was 37a5a2ce48664fccf03ebdffe8999fca714220da.

dmtroyer commented 8 years ago

Was there a --force update on master?

chadwhitacre commented 8 years ago

Was there a --force update on master?

@dmtroyer Not that I'm aware of. I force-pushed the token-auth branch to get this PR out of conflict with master.

dmtroyer commented 8 years ago

Ok, weird. I checked out master and pulled and had a bunch of conflicts. All good now.

chadwhitacre commented 8 years ago

Ok, weird. I checked out master and pulled and had a bunch of conflicts. All good now.

Okay, cool.

How can I help here? Or is there something better for me to work on?

dmtroyer commented 8 years ago

How can I help here? Or is there something better for me to work on?

I think right now it is unfortunately at a place that is hard to break out into smaller discrete items. Maybe once we get the login merged it would make sense to start breaking it out into smaller pieces.

chadwhitacre commented 8 years ago

@dmtroyer Okay, I'm going to familiarize myself with what we have now and with Angular in general.

dmtroyer commented 8 years ago

OK! This bad boy's ready for review. Token auth is wired up for both sides, the only thing implemented on the Angular UI Side is the login page at #/users/login. If you want to do anything with that authentication, it will take a bit of a read up on ng-token-auth if you beat me to establishing that pattern.

MatthewVita commented 8 years ago

Looking good, @dmtroyer! I just threw this together real quick to make use of a directive and a generic auth service: https://github.com/saxifrage/cityasacampus/commit/49b83fd910e3937910c03de5bf1ad42c8fd90412

Are you okay with these tweaks?

Also, since Chad and I just merged our Heroku branch, this branch is going to need some love to catch up with master... whoops :+1:

MatthewVita commented 8 years ago

ahh... should have named 'AuthService' not 'LoginService'. Will amend that soon going to bed now

dmtroyer commented 8 years ago

@MatthewVita I can make the change and rebase.

chadwhitacre commented 8 years ago

the only thing implemented on the Angular UI Side is the login page at #/users/login.

How can I test this out, @dmtroyer? When I navigate to /#/users/login I get a blank page.

screen shot 2015-12-04 at 2 19 22 pm

dmtroyer commented 8 years ago

@whit537 did you run gulp on this branch?

dmtroyer commented 8 years ago

You'll also have to run npm i and bundle install to install some dependencies

chadwhitacre commented 8 years ago

Yup. Ran all three.

chadwhitacre commented 8 years ago

Aaaaand I have a login form now. :-)

screen shot 2015-12-04 at 2 43 58 pm

chadwhitacre commented 8 years ago

I'm seeing [...] auth_headers=%7B%22access-token%22[...] in the request cookie.

MatthewVita commented 8 years ago

I'm seeing [...] auth_headers=%7B%22access-token%22[...] in the request cookie.

Yep. Would prefer if this library used Session Storage, but oh well.

@dmtroyer: can I merge this ? UPDATE: I forgot to add that we may want to merge this now, as is, and create separate PRs/Issues for forgot password, logout, password reset, etc. This PR is the base for all of that.

MatthewVita commented 8 years ago

also, was able to verify that post-login, the library intercepts all Angular $http requests to provide the token. :+1:

dmtroyer commented 8 years ago

Yep. Would prefer if this library used Session Storage, but oh well.

I believe it can be configured to use local storage instead of cookies. Something has to be stored client-side.

I forgot to add that we may want to merge this now, as is, and create separate PRs/Issues for forgot password, logout, password reset, etc. This PR is the base for all of that.

Exactly. I started to work on password reset and registration and realized that extended beyond the scope of this PR. I'll start some new prs for those with the work I started once I get a chance this weekend.

can I merge this ?

Ready to go as far as I'm concerned.

MatthewVita commented 8 years ago

I believe it can be configured to use local storage instead of cookies.

Local storage is better but the best practice is session storage. If it's trivial to do, I'll put in a PR for the library.

Ready to go as far as I'm concerned.

Woo

chadwhitacre commented 8 years ago

!m @dmtroyer @MatthewVita

MatthewVita commented 8 years ago

Local storage is better but the best practice is session storage. If it's trivial to do, I'll put in a PR for the library.

I'm going to do this work here: https://github.com/saxifrage/cityasacampus/issues/370