Oauth interfacing with Github and Google

Overdrivr / micro-ci

Continuous Integration for embedded platforms

0 stars 0 forks source link

Oauth interfacing with Github and Google #14

Closed Overdrivr closed 8 years ago

Overdrivr commented 8 years ago

Fixed #3 Please not that the received tokens are not persisted in the database yet. Connection needs to be re-established every time server is restarted. Opened dedicated ticket #13 for this feature.

DanFaudemer commented 8 years ago

The Oauth a big PR well done !

Client side :

It is trange to have login functionnality in the index.jade. Could you create a dedicate file ?

Server side :

server/boot/passport-github.js:

I don't think it is a good idea to have our GITHUB_CLIENT_SECRET inside our repo. If it becomes public everyone will have access to it.
Is this usefull to keep the commented app.post function ?

For google and github passport. Will it be an issue if we are using an public IP than the local IP for deployment ?

Dan

Overdrivr commented 8 years ago

Good catch Dan, I will fix those things.

As for the GITHUB_CLIENT_SECRET I have opened ticket #12 to replace them by environment variables that won't be versionned. For now it's fine but later on they will be removed (and these credentials will be removed from github, google, etc).

DanFaudemer commented 8 years ago

When I tried to do a node . in your branch I get the following issue :

/var/www/micro-ci/node_modules/loopback/lib/registry.js:304

throw new Error('Model not found: ' + modelName); ^ Error: Model not found: Account at Registry.getModel (/var/www/micro-ci/node_modules/loopback/lib/registry.js:304:9) at /var/www/micro-ci/node_modules/loopback-boot/lib/executor.js:217:24 at Array.forEach (native) at defineModels (/var/www/micro-ci/node_modules/loopback-boot/lib/executor.js:212:23) at setupModels (/var/www/micro-ci/node_modules/loopback-boot/lib/executor.js:180:3) at execute (/var/www/micro-ci/node_modules/loopback-boot/lib/executor.js:33:3) at bootLoopBackApp (/var/www/micro-ci/node_modules/loopback-boot/index.js:140:3) at Object. (/var/www/micro-ci/server/server.js:32:1) at Module._compile (module.js:456:26) at Object.Module._extensions..js (module.js:474:10) `

Any idea ?

Overdrivr commented 8 years ago

Yes, it is due to a remain of the Account model that I forgot to remove. It is fixed by c50b450de4cc53164e9de28d05d2841884524af6

This PR is getting quite big. However, I can also implement end to end tests with Protractor. Front-page claims it's for Angular but it is also fully compatible for non angular websites.

Or I can implement e2e tests in another dedicated branch. What do you think ?

DanFaudemer commented 8 years ago

Yes this branch is quite big. I think it will be better to have a dedicated branch for test creation lets go that way. It will be easier for review.

2016-04-13 8:11 GMT+02:00 Rémi Bèges notifications@github.com:

Yes, it is due to a remain of the Account model that I forgot to remove. It is fixed by c50b450 https://github.com/Overdrivr/micro-ci/commit/c50b450de4cc53164e9de28d05d2841884524af6

This PR is getting quite big. However, I can also implement end to end tests with Protractor http://angular.github.io/protractor/#/. Front-page claims it's for Angular but it is also fully compatible for non angular websites.

Or I can implement e2e tests in another dedicated branch. What do you think ?

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/Overdrivr/micro-ci/pull/14#issuecomment-209245319