#184: support for /oauth2/v1/[clientId] requests

src/index.js

@@ -71,7 +71,7 @@ function stopLoading() {
 /* global process: false */
 if (process.env.NODE_ENV !== 'production') {
     // some shortcuts for testing on localhost
-    window.testOAuth = () => location.href = '/oauth2/v1?client_id=ely&redirect_uri=http%3A%2F%2Fely.by&response_type=code&scope=minecraft_server_session';
+    window.testOAuth = () => location.href = '/oauth2/v1/ely?client_id=ely&redirect_uri=http%3A%2F%2Fely.by&response_type=code&scope=minecraft_server_session';
-    window.testOAuthStatic = () => location.href = '/oauth2/v1?client_id=ely&redirect_uri=static_page_with_code&response_type=code&scope=minecraft_server_session';
+    window.testOAuthStatic = () => location.href = '/oauth2/v1/ely?client_id=ely&redirect_uri=static_page_with_code&response_type=code&scope=minecraft_server_session';
-    window.testOAuthStaticCode = () => location.href = '/oauth2/v1?client_id=ely&redirect_uri=static_page&response_type=code&scope=minecraft_server_session';
+    window.testOAuthStaticCode = () => location.href = '/oauth2/v1/ely?client_id=ely&redirect_uri=static_page&response_type=code&scope=minecraft_server_session';
 }

src/routes.js

@@ -45,13 +45,15 @@ export default function routesFactory(store) {
         }
     };
 
+    // TODO: when react-router v3 is out, should update to oauth2(/v1)(/:clientId)
+    // to oauth2(/:version)(/:clientId) with the help of new route matching options
     return (
         <Route path="/" component={RootPage}>
             <IndexRoute component={IndexPage} {...startAuthFlow} />
 
             <Route path="rules" component={RulesPage} />
 
-            <Route path="oauth2(/:version)" component={OAuthInit} {...startAuthFlow} />
+            <Route path="oauth2(/v1)(/:clientId)" component={OAuthInit} {...startAuthFlow} />
 
             <Route path="auth" component={AuthPage}>
                 <Route path="/login" components={new Login()} {...startAuthFlow} />

src/services/authFlow/AuthFlow.js

@@ -95,7 +95,12 @@ export default class AuthFlow {
      * @return {object} - current request object
      */
     getRequest() {
-        return this.currentRequest ? {...this.currentRequest} : {};
+        return {
+            path: '',
+            query: {},
+            params: {},
+            ...this.currentRequest
+        };
     }
 
     /**
@@ -132,11 +137,6 @@ export default class AuthFlow {
         }
 
         switch (path) {
-            case '/oauth2/v1':
-            case '/oauth2':
-                this.setState(new OAuthState());
-                break;
-
             case '/register':
                 this.setState(new RegisterState());
                 break;
@@ -160,6 +160,9 @@ export default class AuthFlow {
 
             default:
                 switch (path.replace(/(.)\/.+/, '$1')) { // use only first part of an url
+                    case '/oauth2':
+                        this.setState(new OAuthState());
+                        break;
                     case '/activation':
                         this.setState(new ActivationState());
                         break;

src/services/authFlow/OAuthState.js

@@ -3,10 +3,10 @@ import CompleteState from './CompleteState';
 
 export default class OAuthState extends AbstractState {
     enter(context) {
-        const {query} = context.getRequest();
+        const {query, params} = context.getRequest();
 
         return context.run('oAuthValidate', {
-            clientId: query.client_id,
+            clientId: query.client_id || params.clientId,
             redirectUrl: query.redirect_uri,
             responseType: query.response_type,
             scope: query.scope,

tests/services/authFlow/AuthFlow.functional.test.js

@@ -33,7 +33,7 @@ describe('AuthFlow.functional', () => {
         navigate = function navigate(path, extra = {}) { // emulates router behaviour
             if (navigate.lastUrl !== path) {
                 navigate.lastUrl = path;
-                flow.handleRequest({path, ...extra}, navigate);
+                flow.handleRequest({path, query: {}, params: {}, ...extra}, navigate);
             }
         };
 
@@ -69,41 +69,36 @@ describe('AuthFlow.functional', () => {
         });
     });
 
-    it('should oauth without any rendering if no acceptance required', () => {
+    describe('oauth', () => {
-        const expectedRedirect = 'foo';
+        it('should oauth without any rendering if no acceptance required', () => {
+            const expectedRedirect = 'foo';
 
-        Object.assign(state, {
+            Object.assign(state, {
-            user: {
+                user: {
-                isGuest: false,
+                    isGuest: false,
-                isActive: true
+                    isActive: true
-            },
+                },
 
-            routing: {
+                auth: {
-                location: {
+                    oauth: {
-                    query: {
+                        clientId: 123
                     }
                 }
-            },
+            });
 
-            auth: {
+            flow.run.onCall(0).returns({then: (fn) => fn()});
-                oauth: {
+            flow.run.onCall(1).returns({then: (fn) => fn({
-                    clientId: 123
+                redirectUri: expectedRedirect
-                }
+            })});
-            }
+
+            navigate('/oauth2');
+
+            expect(flow.run, 'to have calls satisfying', [
+                ['oAuthValidate', {}],
+                ['oAuthComplete', {}],
+                ['redirect', expectedRedirect]
+            ]);
         });
-
-        flow.run.onCall(0).returns({then: (fn) => fn()});
-        flow.run.onCall(1).returns({then: (fn) => fn({
-            redirectUri: expectedRedirect
-        })});
-
-        navigate('/oauth2', {query: {}});
-
-        expect(flow.run, 'to have calls satisfying', [
-            ['oAuthValidate', {}],
-            ['oAuthComplete', {}],
-            ['redirect', expectedRedirect]
-        ]);
     });
 
     describe('/resend-activation #goBack()', () => {
@@ -112,12 +107,6 @@ describe('AuthFlow.functional', () => {
                 isGuest: true,
                 isActive: false
             };
-
-            state.routing = {
-                location: {
-                    pathname: ''
-                }
-            };
         });
 
         it('should goBack to /activation', () => {

tests/services/authFlow/AuthFlow.test.js

@@ -178,6 +178,7 @@ describe('AuthFlow', () => {
             '/accept-rules': LoginState,
             '/oauth/permissions': LoginState,
             '/oauth/finish': LoginState,
+            '/oauth2/v1/foo': OAuthState,
             '/oauth2/v1': OAuthState,
             '/oauth2': OAuthState,
             '/register': RegisterState,
@@ -260,6 +261,18 @@ describe('AuthFlow', () => {
             sinon.stub(flow, 'run').named('flow.run');
         });
 
+        it('should return request with path, query, params', () => {
+            const request = {path: '/'};
+
+            flow.handleRequest(request);
+
+            expect(flow.getRequest(), 'to equal', {
+                ...request,
+                query: {},
+                params: {}
+            });
+        });
+
         it('should return a copy of current request', () => {
             const request = {path: '/', query: {foo: 'bar'}, params: {baz: 'bud'}};
 

tests/services/authFlow/OAuthState.test.js

@@ -30,7 +30,66 @@ describe('OAuthState', () => {
                 state: 'state'
             };
 
-            context.getRequest.returns({query});
+            context.getRequest.returns({query, params: {}});
+
+            expectRun(
+                mock,
+                'oAuthValidate',
+                sinon.match({
+                    clientId: query.client_id,
+                    redirectUrl: query.redirect_uri,
+                    responseType: query.response_type,
+                    scope: query.scope,
+                    state: query.state
+                })
+            ).returns({then() {}});
+
+            state.enter(context);
+        });
+
+        it('should support clientId through route params', () => {
+            const clientId = 'client_id';
+            const query = {
+                redirect_uri: 'redirect_uri',
+                response_type: 'response_type',
+                scope: 'scope',
+                state: 'state'
+            };
+
+            context.getRequest.returns({
+                query,
+                params: {clientId}
+            });
+
+            expectRun(
+                mock,
+                'oAuthValidate',
+                sinon.match({
+                    clientId,
+                    redirectUrl: query.redirect_uri,
+                    responseType: query.response_type,
+                    scope: query.scope,
+                    state: query.state
+                })
+            ).returns({then() {}});
+
+            state.enter(context);
+        });
+
+        it('should give preference to client_id from query', () => {
+            const clientId = 'wrong_id';
+            const query = {
+                client_id: 'client_id',
+                redirect_uri: 'redirect_uri',
+                response_type: 'response_type',
+                scope: 'scope',
+                state: 'state'
+            };
+
+            context.getRequest.returns({
+                query,
+                params: {clientId}
+            });
 
             expectRun(
                 mock,
@@ -50,7 +109,7 @@ describe('OAuthState', () => {
         it('should transition to complete state on success', () => {
             const promise = Promise.resolve();
 
-            context.getRequest.returns({query: {}});
+            context.getRequest.returns({query: {}, params: {}});
 
             mock.expects('run').returns(promise);
             expectState(mock, CompleteState);