Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authenticating with Google #54

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Conversation

MrFjellstad
Copy link
Contributor

Finally! Rewrote it to work with the new code. Think I have fixed all the comments!

Need to serve the static files trough the node server to get stuff to work...

Missing stuff:
Store the credentials in the database so we can use them for something interesting NSA style!

@petteraas
Copy link
Contributor

Looks like karma.conf.js needs to load more files.


logs.setupErrorLogger(api);

api.use(express.static("../../static/app"));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we've talked about this part before, and I believe you told me that this didn't imply to serve the entirety of static/ through the node app?

If that is correct we can keep it like this for now ( unless there's a easy way of splitting the parts actually needed out of static/?), but down the road I want to split the repo to make it more visible that service/ is a api and static/ is a client for this api, there shouldn't be any hard connection in the code between the two ( opens the possibility of writing other clients, e.g. native phone app ) ( + stuff like https://david-dm.org/ requires a root directory package.json).
Then I'd like to see a separate server wrapping(and serving) static/ for authentication ( this would be the connectionpoint with google auth etc) and connecting with service/ rather via OAuth server to server.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we need to run the parts we want to authenticate trough the node server.

Maybe it is time to start looking at webtokens...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like the problem is the redirect that google does back to our server after a successful login. It gets confused if the ports doesn't match.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem is of course CORS. We use cookie authentication, and that doesn't work well with different ports!

@plumpNation
Copy link
Contributor

Awesome stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants