From c28d066755234ded890743bf986ae3e56d99b8ea Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Wed, 19 Dec 2018 18:39:04 +0100 Subject: [PATCH 1/5] update index.html to accept server values Signed-off-by: Maxim Sukharev --- frontend/public/index.html | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frontend/public/index.html b/frontend/public/index.html index 9cef9300..a72ca3ee 100644 --- a/frontend/public/index.html +++ b/frontend/public/index.html @@ -26,6 +26,7 @@ You need to enable JavaScript to run this app.
+ + +
From ea23963ab0d18dbb59f222b8824e0d881c684ce0 Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Thu, 20 Dec 2018 16:18:37 +0100 Subject: [PATCH 2/5] add /api/me endpoint Signed-off-by: Maxim Sukharev --- web/auth.go | 99 +++++++++++++++++++++++++++++++++++----------- web/auth_test.go | 11 ++++-- web/http_server.go | 9 ++++- 3 files changed, 89 insertions(+), 30 deletions(-) diff --git a/web/auth.go b/web/auth.go index 6f55a999..34e556b3 100644 --- a/web/auth.go +++ b/web/auth.go @@ -17,6 +17,38 @@ import ( "golang.org/x/oauth2/github" ) +// TODO: move/rewrite it when we have other endpoints + +type successResp struct { + Data interface{} `json:"data"` +} +type errorResp struct { + Errors []error `json:"errors"` +} + +func successJSON(w http.ResponseWriter, r *http.Request, data interface{}) { + b, err := json.Marshal(successResp{data}) + if err != nil { + lg.RequestLog(r).Warn(err.Error()) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + + w.Write(b) +} + +func errorJSON(w http.ResponseWriter, r *http.Request, code int, errors ...error) { + b, err := json.Marshal(errorResp{errors}) + if err != nil { + lg.RequestLog(r).Warn(err.Error()) + http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + return + } + + w.WriteHeader(code) + w.Write(b) +} + // Auth is http service to authorize users, it uses oAuth and JWT underneath type Auth struct { config *oauth2.Config @@ -87,28 +119,44 @@ func (a *Auth) Callback(w http.ResponseWriter, r *http.Request) { return } - user, err := a.getUser(r.Context(), code) + oauthToken, err := a.config.Exchange(r.Context(), code) + if err != nil { + http.Error(w, fmt.Sprintf("oauth exchange error: %s", err), http.StatusBadRequest) + return + } + + user, err := a.getUser(r.Context(), oauthToken) if err != nil { lg.RequestLog(r).Warn(fmt.Errorf("oauth get user error: %s", err)) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - token, err := a.makeToken(user) + token, err := a.makeToken(*oauthToken, user) if err != nil { lg.RequestLog(r).Warn(fmt.Errorf("make jwt token error: %s", err)) http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) return } - b, err := json.Marshal(callbackResp{token}) + successJSON(w, r, callbackResp{token}) +} + +// Me endpoint make request to provider and returns user details +func (a *Auth) Me(w http.ResponseWriter, r *http.Request) { + t, err := getOAuthToken(r.Context()) if err != nil { - lg.RequestLog(r).Warn(err.Error()) - http.Error(w, http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError) + http.Error(w, err.Error(), http.StatusInternalServerError) return } - w.Write(b) + u, err := a.getUser(r.Context(), t) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + successJSON(w, r, u) } // Middleware return http.Handler which validates token and set user id in context @@ -122,8 +170,10 @@ func (a *Auth) Middleware(next http.Handler) http.Handler { w.WriteHeader(http.StatusUnauthorized) return } - r = r.WithContext(SetUserID(r.Context(), claims.ID)) - next.ServeHTTP(w, r) + + ctx := context.WithValue(r.Context(), userIDKey, claims.ID) + ctx = context.WithValue(ctx, userOAuthToken, &claims.OAuthToken) + next.ServeHTTP(w, r.WithContext(ctx)) }) } @@ -162,12 +212,7 @@ func (a *Auth) validateState(r *http.Request, state string) error { } // getUser gets user from provider and return user model -func (a *Auth) getUser(ctx context.Context, code string) (*User, error) { - token, err := a.config.Exchange(ctx, code) - if err != nil { - return nil, fmt.Errorf("oauth exchange error: %s", err) - } - +func (a *Auth) getUser(ctx context.Context, token *oauth2.Token) (*User, error) { return a.userGetter(a.config.Client(ctx, token)) } @@ -188,13 +233,14 @@ func getGithubUser(client *http.Client) (*User, error) { } type jwtClaim struct { - ID int + ID int + OAuthToken oauth2.Token jwt.StandardClaims } // makeToken generates token string for a user -func (a *Auth) makeToken(user *User) (string, error) { - claims := &jwtClaim{ID: user.ID} +func (a *Auth) makeToken(ot oauth2.Token, user *User) (string, error) { + claims := &jwtClaim{ID: user.ID, OAuthToken: ot} t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims) ss, err := t.SignedString(a.signingKey) if err != nil { @@ -220,9 +266,10 @@ var extractor = &request.PostExtractionFilter{ Filter: stripBearerPrefixFromTokenString, } -type userIDContext int +type userContext int -const userIDKey userIDContext = 1 +const userIDKey userContext = 1 +const userOAuthToken userContext = 2 // getUserInt gets the value stored in the Context for the key userIDKey, bool // is true on success @@ -231,11 +278,6 @@ func getUserInt(ctx context.Context) (int, bool) { return i, ok } -// SetUserID sets the user ID to the context -func SetUserID(ctx context.Context, userID int) context.Context { - return context.WithValue(ctx, userIDKey, userID) -} - // GetUserID gets the user ID set by the JWT middleware in the Context func GetUserID(ctx context.Context) (int, error) { id, ok := getUserInt(ctx) @@ -245,3 +287,12 @@ func GetUserID(ctx context.Context) (int, error) { return id, nil } + +func getOAuthToken(ctx context.Context) (*oauth2.Token, error) { + t, ok := ctx.Value(userOAuthToken).(*oauth2.Token) + if !ok { + return nil, fmt.Errorf("OAuth token is not set in the context") + } + + return t, nil +} diff --git a/web/auth_test.go b/web/auth_test.go index 95a5da36..e1cf6bf1 100644 --- a/web/auth_test.go +++ b/web/auth_test.go @@ -81,14 +81,17 @@ func TestCallbackSuccess(t *testing.T) { require.Equal(http.StatusOK, w.Code) var resp struct { - Token string + Data struct { + Token string + } } err := json.Unmarshal(w.Body.Bytes(), &resp) require.NoError(err) - token, err := auth.makeToken(testUser) + oauthToken := oauth2.Token{AccessToken: "access-token"} + token, err := auth.makeToken(oauthToken, testUser) require.NoError(err) - require.Equal(resp.Token, token) + require.Equal(token, resp.Data.Token) } func TestMiddlewareSuccess(t *testing.T) { @@ -106,7 +109,7 @@ func TestMiddlewareSuccess(t *testing.T) { require.Equal(testUser.ID, id) })) - token, err := auth.makeToken(testUser) + token, err := auth.makeToken(oauth2.Token{}, testUser) require.NoError(err) w := httptest.NewRecorder() diff --git a/web/http_server.go b/web/http_server.go index e7c9ecbe..b97df22a 100644 --- a/web/http_server.go +++ b/web/http_server.go @@ -16,7 +16,9 @@ type HTTPServer struct { func NewHTTPServer(auth *Auth, static *Static) *HTTPServer { corsOptions := cors.Options{ - AllowedOrigins: []string{"*"}, + // TODO: make it customizable + // we can't pass "*" because it's incompatible with "credentials: include" request + AllowedOrigins: []string{"http://127.0.0.1:3000"}, AllowedMethods: []string{"GET", "POST", "PUT", "OPTIONS"}, AllowedHeaders: []string{"Location", "Authorization", "Content-Type"}, AllowCredentials: true, @@ -32,7 +34,10 @@ func NewHTTPServer(auth *Auth, static *Static) *HTTPServer { r.Use(middleware.Recoverer) r.Get("/login", auth.Login) - r.Get("/callback", auth.Callback) + r.Get("/api/callback", auth.Callback) + r.With(auth.Middleware).Route("/api", func(r chi.Router) { + r.Get("/me", auth.Me) + }) r.Get("/static/*", static.ServeHTTP) r.Get("/*", static.ServeHTTP) From c87ec0939971581a5e9841faaabcc07b2ea1b1c7 Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Wed, 19 Dec 2018 18:51:10 +0100 Subject: [PATCH 3/5] implement initial frontend for authorization Signed-off-by: Maxim Sukharev --- frontend/src/App.tsx | 111 ++++++++++++++++++++++++++++--- frontend/src/api.ts | 80 ++++++++++++++++++++++ frontend/src/services/options.ts | 11 +++ frontend/src/services/token.ts | 21 ++++++ 4 files changed, 212 insertions(+), 11 deletions(-) create mode 100644 frontend/src/api.ts create mode 100644 frontend/src/services/options.ts create mode 100644 frontend/src/services/token.ts diff --git a/frontend/src/App.tsx b/frontend/src/App.tsx index c18c22d8..fbcc537d 100644 --- a/frontend/src/App.tsx +++ b/frontend/src/App.tsx @@ -1,17 +1,106 @@ -import React, { Component } from 'react'; +import React, { Component, ReactElement } from 'react'; +import Token from './services/token'; +import * as api from './api'; import './App.css'; -class App extends Component { +function Loader() { + return
loading...
; +} + +interface ErrorProps { + errors: string[]; +} + +function Errors({ errors }: ErrorProps) { + return
{errors.join(',')}
; +} + +function Login() { + return ( +
+ + Login using Github + +
+ ); +} + +interface HelloProps { + name: string; +} + +function Hello({ name }: HelloProps) { + return
Hello {name}!
; +} + +interface AppState { + // we need undefined state for initial render + loggedIn: boolean | undefined; + name: string; + errors: string[]; +} + +class App extends Component<{}, AppState> { + constructor(props: {}) { + super(props); + + this.fetchState = this.fetchState.bind(this); + + this.state = { + loggedIn: undefined, + name: '', + errors: [] + }; + } + + componentDidMount() { + // TODO: add router and use it instead of this "if" + if (window.location.pathname === '/callback') { + api + .callback(window.location.search) + .then(resp => { + Token.set(resp.token); + window.history.replaceState({}, '', '/'); + }) + .then(this.fetchState) + .catch(errors => this.setState({ errors })); + return; + } + + if (!Token.exists()) { + this.setState({ loggedIn: false }); + return; + } + + // ignore error here, just ask user to re-login + // it would cover all cases like expired token, changes on backend and so on + this.fetchState().catch(err => console.error(err)); + } + + fetchState() { + return api + .me() + .then(resp => this.setState({ loggedIn: true, name: resp.name })) + .catch(err => { + this.setState({ loggedIn: false }); + + throw err; + }); + } + render() { - return ( - - ); + const { loggedIn, name, errors } = this.state; + + let content: ReactElement; + if (errors.length) { + content = ; + } else if (typeof loggedIn === 'undefined') { + content = ; + } else { + content = loggedIn ? : ; + } + + return
{content}
; } } diff --git a/frontend/src/api.ts b/frontend/src/api.ts new file mode 100644 index 00000000..bf7824d8 --- /dev/null +++ b/frontend/src/api.ts @@ -0,0 +1,80 @@ +import lookoutOptions from './services/options'; +import TokenService from './services/token'; + +export const serverUrl = lookoutOptions.SERVER_URL || 'http://127.0.0.1:8080'; + +const apiUrl = (url: string) => `${serverUrl}${url}`; + +interface ApiCallOptions { + method?: string; + body?: object; +} + +interface ServerError { + title: string; + description: string; +} + +function apiCall(url: string, options: ApiCallOptions = {}): Promise { + const token = TokenService.get(); + const fetchOptions: RequestInit = { + credentials: 'include', + headers: { + Authorization: `Bearer ${token}`, + 'Content-Type': 'application/json' + }, + body: null + }; + + if (options.body) { + fetchOptions.body = JSON.stringify(options.body); + } + + return fetch(apiUrl(url), fetchOptions).then(response => { + if (!response.ok) { + // when server return Unauthorized we need to remove token + if (response.status === 401) { + TokenService.remove(); + } + + return response + .json() + .catch(() => { + throw [response.statusText]; + }) + .then(json => { + let errors: string[]; + + try { + errors = (json as { errors: ServerError[] }).errors.map( + e => e.title + ); + } catch (e) { + errors = [e.toString()]; + } + + throw errors; + }); + } + + return response.json().then(json => (json as { data: T }).data); + }); +} + +export const loginUrl = apiUrl('/login'); + +interface AuthResponse { + token: string; +} + +export function callback(queryString: string): Promise { + return apiCall(`/api/callback${queryString}`); +} + +interface MeResponse { + name: string; +} + +export function me(): Promise { + return apiCall('/api/me'); +} diff --git a/frontend/src/services/options.ts b/frontend/src/services/options.ts new file mode 100644 index 00000000..ee100ccc --- /dev/null +++ b/frontend/src/services/options.ts @@ -0,0 +1,11 @@ +interface LookoutApiOptions { + SERVER_URL?: string; +} + +declare global { + interface Window { + lookout: LookoutApiOptions; + } +} + +export default window.lookout || {}; diff --git a/frontend/src/services/token.ts b/frontend/src/services/token.ts new file mode 100644 index 00000000..cd036e02 --- /dev/null +++ b/frontend/src/services/token.ts @@ -0,0 +1,21 @@ +const localStorageKey = 'token'; + +class TokenService { + get() { + return window.localStorage.getItem(localStorageKey); + } + + set(token: string) { + return window.localStorage.setItem(localStorageKey, token); + } + + remove() { + return window.localStorage.removeItem(localStorageKey); + } + + exists() { + return !!window.localStorage.getItem(localStorageKey); + } +} + +export default new TokenService(); From 6779b56e1dcf48caec20c9b108a4708ed2160ccd Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Wed, 26 Dec 2018 11:40:19 +0100 Subject: [PATCH 4/5] add tests for api layer Signed-off-by: Maxim Sukharev --- frontend/package.json | 5 ++- frontend/src/api.test.tsx | 67 +++++++++++++++++++++++++++++++++++++++ frontend/src/api.ts | 5 ++- frontend/yarn.lock | 31 ++++++++++++++++++ 4 files changed, 106 insertions(+), 2 deletions(-) create mode 100644 frontend/src/api.test.tsx diff --git a/frontend/package.json b/frontend/package.json index 064a6618..c43f2d31 100644 --- a/frontend/package.json +++ b/frontend/package.json @@ -26,5 +26,8 @@ "not dead", "not ie <= 11", "not op_mini all" - ] + ], + "devDependencies": { + "jest-fetch-mock": "^2.1.0" + } } diff --git a/frontend/src/api.test.tsx b/frontend/src/api.test.tsx new file mode 100644 index 00000000..ba1c2e82 --- /dev/null +++ b/frontend/src/api.test.tsx @@ -0,0 +1,67 @@ +import { GlobalWithFetchMock } from 'jest-fetch-mock'; +import Token from './services/token'; +import { apiCall } from './api'; + +// can be moved to setupFiles later if needed +const customGlobal: GlobalWithFetchMock = global as GlobalWithFetchMock; +customGlobal.fetch = require('jest-fetch-mock'); +customGlobal.fetchMock = customGlobal.fetch; + +describe('api', () => { + beforeEach(() => { + fetchMock.resetMocks(); + }); + + it('apiCall ok', () => { + Token.set('token'); + fetchMock.mockResponseOnce(JSON.stringify({ data: 'result' })); + + return apiCall('/test').then(resp => { + expect(resp).toEqual('result'); + + const call = fetchMock.mock.calls[0]; + const [url, opts] = call; + expect(url).toEqual('http://127.0.0.1:8080/test'); + expect(opts.headers.Authorization).toEqual('Bearer token'); + }); + }); + + it('apiCall http error', () => { + fetchMock.mockResponseOnce('', { status: 500 }); + + return apiCall('/test').catch(err => { + expect(err).toEqual(['Internal Server Error']); + }); + }); + + it('apiCall http error with custom text', () => { + fetchMock.mockResponseOnce('', { status: 404, statusText: 'Custom text' }); + + return apiCall('/test').catch(err => { + expect(err).toEqual(['Custom text']); + }); + }); + + it('apiCall http error with json response', () => { + fetchMock.mockResponseOnce( + JSON.stringify({ errors: [{ title: 'err1' }, { title: 'err2' }] }), + { + status: 500 + } + ); + + return apiCall('/test').catch(err => { + expect(err).toEqual(['err1', 'err2']); + }); + }); + + it('apiCall removes token on unauthorized response', () => { + Token.set('token'); + fetchMock.mockResponseOnce('', { status: 401 }); + + return apiCall('/test').catch(err => { + expect(err).toEqual(['Unauthorized']); + expect(Token.get()).toBe(null); + }); + }); +}); diff --git a/frontend/src/api.ts b/frontend/src/api.ts index bf7824d8..0470a614 100644 --- a/frontend/src/api.ts +++ b/frontend/src/api.ts @@ -15,7 +15,10 @@ interface ServerError { description: string; } -function apiCall(url: string, options: ApiCallOptions = {}): Promise { +export function apiCall( + url: string, + options: ApiCallOptions = {} +): Promise { const token = TokenService.get(); const fetchOptions: RequestInit = { credentials: 'include', diff --git a/frontend/yarn.lock b/frontend/yarn.lock index 289d7a40..f2f7a5a9 100644 --- a/frontend/yarn.lock +++ b/frontend/yarn.lock @@ -2438,6 +2438,14 @@ create-hmac@^1.1.0, create-hmac@^1.1.2, create-hmac@^1.1.4: safe-buffer "^5.0.1" sha.js "^2.4.8" +cross-fetch@^2.2.2: + version "2.2.3" + resolved "https://registry.yarnpkg.com/cross-fetch/-/cross-fetch-2.2.3.tgz#e8a0b3c54598136e037f8650f8e823ccdfac198e" + integrity sha512-PrWWNH3yL2NYIb/7WF/5vFG3DCQiXDOVf8k3ijatbrtnwNuhMWLC7YF7uqf53tbTFDzHIUD8oITw4Bxt8ST3Nw== + dependencies: + node-fetch "2.1.2" + whatwg-fetch "2.0.4" + cross-spawn@6.0.5, cross-spawn@^6.0.0, cross-spawn@^6.0.5: version "6.0.5" resolved "https://registry.yarnpkg.com/cross-spawn/-/cross-spawn-6.0.5.tgz#4a5ec7c64dfae22c3a14124dbacdee846d80cbc4" @@ -5159,6 +5167,14 @@ jest-environment-node@^23.4.0: jest-mock "^23.2.0" jest-util "^23.4.0" +jest-fetch-mock@^2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/jest-fetch-mock/-/jest-fetch-mock-2.1.0.tgz#49c16451b82f311158ec897e467d704e0cb118f9" + integrity sha512-jrTNlxDsZZCq6tMhdyH7gIbt4iDUHRr6C4Jp+kXItLaaaladOm9/wJjIwU3tCAEohbuW/7/naOSfg2A8H6/35g== + dependencies: + cross-fetch "^2.2.2" + promise-polyfill "^7.1.1" + jest-get-type@^22.1.0: version "22.4.3" resolved "https://registry.yarnpkg.com/jest-get-type/-/jest-get-type-22.4.3.tgz#e3a8504d8479342dd4420236b322869f18900ce4" @@ -6196,6 +6212,11 @@ no-case@^2.2.0: dependencies: lower-case "^1.1.1" +node-fetch@2.1.2: + version "2.1.2" + resolved "https://registry.yarnpkg.com/node-fetch/-/node-fetch-2.1.2.tgz#ab884e8e7e57e38a944753cec706f788d1768bb5" + integrity sha1-q4hOjn5X44qUR1POxwb3iNF2i7U= + node-forge@0.7.5: version "0.7.5" resolved "https://registry.yarnpkg.com/node-forge/-/node-forge-0.7.5.tgz#6c152c345ce11c52f465c2abd957e8639cd674df" @@ -7520,6 +7541,11 @@ promise-inflight@^1.0.1: resolved "https://registry.yarnpkg.com/promise-inflight/-/promise-inflight-1.0.1.tgz#98472870bf228132fcbdd868129bad12c3c029e3" integrity sha1-mEcocL8igTL8vdhoEputEsPAKeM= +promise-polyfill@^7.1.1: + version "7.1.2" + resolved "https://registry.yarnpkg.com/promise-polyfill/-/promise-polyfill-7.1.2.tgz#ab05301d8c28536301622d69227632269a70ca3b" + integrity sha512-FuEc12/eKqqoRYIGBrUptCBRhobL19PS2U31vMNTfyck1FxPyMfgsXyW4Mav85y/ZN1hop3hOwRlUDok23oYfQ== + promise@8.0.2: version "8.0.2" resolved "https://registry.yarnpkg.com/promise/-/promise-8.0.2.tgz#9dcd0672192c589477d56891271bdc27547ae9f0" @@ -9520,6 +9546,11 @@ whatwg-encoding@^1.0.1, whatwg-encoding@^1.0.3, whatwg-encoding@^1.0.5: dependencies: iconv-lite "0.4.24" +whatwg-fetch@2.0.4: + version "2.0.4" + resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-2.0.4.tgz#dde6a5df315f9d39991aa17621853d720b85566f" + integrity sha512-dcQ1GWpOD/eEQ97k66aiEVpNnapVj90/+R+SXTPYGHpYBBypfKJEQjLrvMZ7YXbKm21gXd4NcuxUTjiv1YtLng== + whatwg-fetch@3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/whatwg-fetch/-/whatwg-fetch-3.0.0.tgz#fc804e458cc460009b1a2b966bc8817d2578aefb" From 676db5da17c999fbefc081e7e8b51dae1385baa4 Mon Sep 17 00:00:00 2001 From: Maxim Sukharev Date: Wed, 26 Dec 2018 16:29:27 +0100 Subject: [PATCH 5/5] clarify the value of AllowedOrigins in a comment Signed-off-by: Maxim Sukharev --- web/http_server.go | 1 + 1 file changed, 1 insertion(+) diff --git a/web/http_server.go b/web/http_server.go index b97df22a..e6816e82 100644 --- a/web/http_server.go +++ b/web/http_server.go @@ -18,6 +18,7 @@ func NewHTTPServer(auth *Auth, static *Static) *HTTPServer { corsOptions := cors.Options{ // TODO: make it customizable // we can't pass "*" because it's incompatible with "credentials: include" request + // http://127.0.0.1:3000 is the default url of create-react-app dev server AllowedOrigins: []string{"http://127.0.0.1:3000"}, AllowedMethods: []string{"GET", "POST", "PUT", "OPTIONS"}, AllowedHeaders: []string{"Location", "Authorization", "Content-Type"},