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

First party login #101

Merged
merged 9 commits into from
Jul 16, 2014
Merged

First party login #101

merged 9 commits into from
Jul 16, 2014

Conversation

edpaget
Copy link
Contributor

@edpaget edpaget commented Jul 15, 2014

Hey all, this should handle user login and retrieving the OAuth token for sites on the *.zooniverse.org domain.

Basically the main thrust is that it allows a devise session cookie to be substituted for users credentials when making a request to the /oauth/token endpoint. So if a client is on a *.zooniverse.org domain, and a user is signed in using devise sending a request like...

{
   "grant_type": "password",
   "client_id": "asdk;fja;sdfqwklj;laskdfja;sdlfkj..."
}

...returns a fully scoped OAuth token for the site.

It also adds support for managing a devise session over JSON.

The only caveat is all of those endpoints need to use Rails's CSRF protection features to prevent a third-party site from ohijacking a users's session and getting free oauth tokens. Therefore the javascript client will need to look for the X-CSRF-Param and X-CSRF-Token headers and respond with a X-CSRF-Token header.

@@ -4,18 +4,13 @@
controllers applications: 'oauth/applications'
end

devise_for :users, controllers: { omniauth_callbacks: 'omniauth_callbacks' },
skip: [ :registrations, :sessions, :passwords ]
devise_for :users, controllers: { omniauth_callbacks: 'omniauth_callbacks',
Copy link
Contributor

Choose a reason for hiding this comment

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

These provide all the route functionality for devise. We will need most of them (now we're offering HTML views for user control, etc) but routes like GET /users/cancel and DELETE /users are already implemented in the api user routes.

If we leave them as is then actual user delete functionality will exist and not the desired disable as we implemented via DELETE /api/users/. Previously I added just the routes we wanted to respond to, all others would raise a Routing Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool, I will override those actions or remove those routes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@camallen does 61970fd resolve this for you?

context "as json" do
describe "#create" do
before(:each) do
request.env["HTTP_ACCEPT"] = "application/vnd.api+json; version=1"
Copy link
Contributor

Choose a reason for hiding this comment

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

remove the version string here

camallen added a commit that referenced this pull request Jul 16, 2014
@camallen camallen merged commit 26d244c into zooniverse:master Jul 16, 2014
@edpaget edpaget deleted the first-party-login branch July 16, 2014 16:29
@camallen
Copy link
Contributor

nice one @edpaget. @brian-c for logins on the client the workflow for both U&P and facebook would be:

  1. Sign_in via /users/sign_in OR via the facebook omni auth callback to get a devise session.
  2. Get a token via /oauth/tokens with the params
{
   "grant_type": "password",
   "client_id": "application_client_id_but_no_secret"
}
  1. Use token to access the API routes with the custom versioned json application header.

@edpaget
Copy link
Contributor Author

edpaget commented Jul 16, 2014

You can also GET /users/sign_in to get a json list of all possible auth methods and their URLs.

camallen added a commit to camallen/Panoptes that referenced this pull request Jul 26, 2018
ensure the spec testing devise session to token via password grant (zooniverse#101) uses a non-confidential application
camallen pushed a commit that referenced this pull request Aug 7, 2018
* [Security] Bump doorkeeper from 4.2.6 to 4.4.0

Bumps [doorkeeper](https://github.com/doorkeeper-gem/doorkeeper) from 4.2.6 to 4.4.0. **This update includes security fixes.**
- [Release notes](https://github.com/doorkeeper-gem/doorkeeper/releases)
- [Changelog](https://github.com/doorkeeper-gem/doorkeeper/blob/master/NEWS.md)
- [Commits](doorkeeper-gem/doorkeeper@v4.2.6...v4.4.0)

Signed-off-by: dependabot[bot] <support@dependabot.com>

* add confidential column to oauth_applications

WARNING: This is a security release that addresses token revocation not working for public apps (CVE-2018-1000211)

There is no breaking change in this release, however to take advantage of the security fix you must:

  1. Run `rails generate doorkeeper:add_client_confidentiality` for the migration
  2. Review your OAuth apps and determine which ones exclusively use public grant flows (eg implicit)
  3. Update their `confidential` column to `false` for those public apps

This is a backported security release.

  For more information:

    * doorkeeper-gem/doorkeeper#1119
    * doorkeeper-gem/doorkeeper#891

* add note on what this migration default value means

some of our apps (first_party password / session grants and implicit) will require the non-default value of confidential = false

* switch to non-confidential oauth application

ensure the spec testing devise session to token via password grant (#101) uses a non-confidential application

* add data migration for oauth confidential attribute

set confidential to false for implicit, native apps as well as any first party apps that use the password grant without supplying the client secret (PFE / python client).

* move comment to be more informative

* only change apps we know use non-confidential password flow

avoid changing other apps that may be using client_id & client_secrets to authenticate, only update the first party apps we know about.
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.

2 participants