diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index acd0b8b..45e832e 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -91,7 +91,7 @@ If you want to benefit from frontend hot reloading feature: Run server. Execute: ```bash -$ UI_DOMAIN=http://127.0.0.1:3000 make gorun +$ make gorun ``` And then run frontend in dev mode. Execute: diff --git a/README.md b/README.md index f1f4056..1680ac3 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,6 @@ The next sections make use of several environment variables to configure the app | `CAT_HOST` | | `0.0.0.0` | IP address to bind the HTTP server | | `CAT_PORT` | | `8080` | Port address to bind the HTTP server | | `CAT_SERVER_URL` | | `:` | URL used to access the application (i.e. public hostname) | -| `CAT_UI_DOMAIN` | | `:` | URL used to access the frontend development server | | `CAT_DB_CONNECTION` | | `sqlite:///var/code-annotation/internal.db` | Points to the internal application database. [Read below](#importing-and-exporting-data) for the complete syntax | | `CAT_EXPORTS_PATH` | | `./exports` | Folder where the SQLite files will be created when requested from `http:///export` | | `CAT_ENV` | | `production` | Sets the log level. Use `dev` to enable debug log messages | diff --git a/cli/server/server.go b/cli/server/server.go index f8206f0..f0d31ed 100644 --- a/cli/server/server.go +++ b/cli/server/server.go @@ -21,7 +21,6 @@ type appConfig struct { Host string `envconfig:"HOST" default:"0.0.0.0"` Port int `envconfig:"PORT" default:"8080"` ServerURL string `envconfig:"SERVER_URL"` - UIDomain string `envconfig:"UI_DOMAIN"` DBConn string `envconfig:"DB_CONNECTION" default:"sqlite:///var/code-annotation/internal.db"` ExportsPath string `envconfig:"EXPORTS_PATH" default:"./exports"` GaTrackingID string `envconfig:"GA_TRACKING_ID" required:"false"` @@ -34,9 +33,6 @@ func main() { if conf.ServerURL == "" { conf.ServerURL = fmt.Sprintf("//%s:%d", conf.Host, conf.Port) } - if conf.UIDomain == "" { - conf.UIDomain = fmt.Sprintf("//%s:%d", conf.Host, conf.Port) - } // loger logger := service.NewLogger(conf.Env) @@ -73,7 +69,7 @@ func main() { static := handler.NewStatic("build", conf.ServerURL, conf.GaTrackingID) // start the router - router := server.Router(logger, jwt, oauth, diffService, static, conf.UIDomain, &db, conf.ExportsPath, version) + router := server.Router(logger, jwt, oauth, diffService, static, &db, conf.ExportsPath, version) logger.Info("running...") err = http.ListenAndServe(fmt.Sprintf("%s:%d", conf.Host, conf.Port), router) logger.Fatal(err) diff --git a/helm-charts/code-annotation/templates/deployment.yaml b/helm-charts/code-annotation/templates/deployment.yaml index 982a362..4a772f3 100644 --- a/helm-charts/code-annotation/templates/deployment.yaml +++ b/helm-charts/code-annotation/templates/deployment.yaml @@ -32,8 +32,6 @@ spec: value: "{{ .Values.authorization.restrictRequesterGroup }}" - name: CAT_SERVER_URL value: "//{{ .Values.ingress.hostname }}" - - name: CAT_UI_DOMAIN - value: "//{{ .Values.ingress.hostname }}" - name: CAT_DB_CONNECTION value: "sqlite://{{ .Values.deployment.internalDatabasePath }}/internal.db" - name: CAT_OAUTH_CLIENT_ID diff --git a/server/handler/auth.go b/server/handler/auth.go index 8a7f165..06a6b1b 100644 --- a/server/handler/auth.go +++ b/server/handler/auth.go @@ -9,13 +9,21 @@ import ( "github.com/src-d/code-annotation/server/serializer" "github.com/src-d/code-annotation/server/service" + "github.com/pressly/lg" "github.com/sirupsen/logrus" ) // Login handler redirects user to oauth provider func Login(oAuth *service.OAuth) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { - url := oAuth.MakeAuthURL(w, r) + url, err := oAuth.MakeAuthURL(w, r) + if err != nil { + lg.RequestLog(r).Warn(err.Error()) + http.Error(w, + http.StatusText(http.StatusInternalServerError), http.StatusInternalServerError, + ) + } + http.Redirect(w, r, url, http.StatusTemporaryRedirect) } } @@ -25,51 +33,44 @@ func OAuthCallback( oAuth *service.OAuth, jwt *service.JWT, userRepo *repository.Users, - uiDomain string, logger logrus.FieldLogger, -) http.HandlerFunc { - return func(w http.ResponseWriter, r *http.Request) { - if err := oAuth.ValidateState(r, r.FormValue("state")); err != nil { - errorText := "The state passed is incorrect or expired" - write( - w, r, - serializer.NewEmptyResponse(), - serializer.NewHTTPError(http.StatusBadRequest, errorText), +) RequestProcessFunc { + return func(r *http.Request) (*serializer.Response, error) { + state := r.URL.Query().Get("state") + if err := oAuth.ValidateState(r, state); err != nil { + logger.Warn(err) + return nil, serializer.NewHTTPError( + http.StatusPreconditionFailed, + "The state passed by github is incorrect or expired", ) - return } - code := r.FormValue("code") + code := r.URL.Query().Get("code") if code == "" { - errorText := r.FormValue("error_description") + errorText := r.URL.Query().Get("error_description") if errorText == "" { errorText = "OAuth provided didn't send code in callback" } - write( - w, r, - serializer.NewEmptyResponse(), - serializer.NewHTTPError(http.StatusBadRequest, errorText), - ) - return + + return nil, serializer.NewHTTPError(http.StatusBadRequest, errorText) } ghUser, err := oAuth.GetUser(r.Context(), code) if err == service.ErrNoAccess { - http.Error(w, http.StatusText(http.StatusForbidden), http.StatusForbidden) - return + return nil, serializer.NewHTTPError( + http.StatusForbidden, + http.StatusText(http.StatusForbidden), + ) } + if err != nil { - logger.Errorf("oauth get user error: %s", err) // FIXME can it be not server error? for wrong code - write(w, r, serializer.NewEmptyResponse(), err) - return + return nil, fmt.Errorf("oauth get user error: %s", err) } user, err := userRepo.Get(ghUser.Login) if err != nil { - logger.Error(err) - write(w, r, serializer.NewEmptyResponse(), err) - return + return nil, fmt.Errorf("get user from db: %s", err) } if user == nil { @@ -82,9 +83,7 @@ func OAuthCallback( err = userRepo.Create(user) if err != nil { - logger.Errorf("can't create user: %s", err) - write(w, r, serializer.NewEmptyResponse(), err) - return + return nil, fmt.Errorf("can't create user: %s", err) } } else { user.Username = ghUser.Username @@ -92,19 +91,15 @@ func OAuthCallback( user.Role = ghUser.Role if err = userRepo.Update(user); err != nil { - logger.Errorf("can't update user: %s", err) - write(w, r, serializer.NewEmptyResponse(), err) - return + return nil, fmt.Errorf("can't update user: %s", err) } } token, err := jwt.MakeToken(user) if err != nil { - logger.Errorf("make jwt token error: %s", err) - write(w, r, serializer.NewEmptyResponse(), err) - return + return nil, fmt.Errorf("make jwt token error: %s", err) } - url := fmt.Sprintf("%s/?token=%s", uiDomain, token) - http.Redirect(w, r, url, http.StatusTemporaryRedirect) + + return serializer.NewTokenResponse(token), nil } } diff --git a/server/handler/render.go b/server/handler/render.go index 542a6de..a8768b2 100644 --- a/server/handler/render.go +++ b/server/handler/render.go @@ -6,9 +6,10 @@ import ( "net/http" "strconv" + "github.com/src-d/code-annotation/server/serializer" + "github.com/go-chi/chi" "github.com/pressly/lg" - "github.com/src-d/code-annotation/server/serializer" ) // RequestProcessFunc is a function that takes an http.Request, and returns a serializer.Response and an error diff --git a/server/router.go b/server/router.go index fd640ed..d3ff6d2 100644 --- a/server/router.go +++ b/server/router.go @@ -23,7 +23,6 @@ func Router( oauth *service.OAuth, diffService *service.Diff, static *handler.Static, - uiDomain string, dbWrapper *dbutil.DB, exportsPath string, version string, @@ -40,9 +39,10 @@ func Router( // cors options corsOptions := cors.Options{ - AllowedOrigins: []string{"*"}, - AllowedMethods: []string{"GET", "POST", "PUT", "OPTIONS"}, - AllowedHeaders: []string{"Location", "Authorization", "Content-Type"}, + AllowedOrigins: []string{"*"}, + AllowedMethods: []string{"GET", "POST", "PUT", "OPTIONS"}, + AllowedHeaders: []string{"Location", "Authorization", "Content-Type"}, + AllowCredentials: true, } requesterACL := service.NewACL(userRepo, model.Requester) @@ -55,7 +55,7 @@ func Router( r.Use(lg.RequestLogger(logger)) r.Get("/login", handler.Login(oauth)) - r.Get("/oauth-callback", handler.OAuthCallback(oauth, jwt, userRepo, uiDomain, logger)) + r.Get("/api/auth", handler.APIHandlerFunc(handler.OAuthCallback(oauth, jwt, userRepo, logger))) r.Route("/api", func(r chi.Router) { r.Use(jwt.Middleware) diff --git a/server/serializer/serializers.go b/server/serializer/serializers.go index a083d17..a400750 100644 --- a/server/serializer/serializers.go +++ b/server/serializer/serializers.go @@ -228,3 +228,12 @@ type filePairsUploadResponse struct { func NewFilePairsUploadResponse(success, failures int64) *Response { return newResponse(filePairsUploadResponse{success, failures}) } + +type tokenResponse struct { + Token string `json:"token"` +} + +// NewTokenResponse returns a Response with a token +func NewTokenResponse(token string) *Response { + return newResponse(tokenResponse{token}) +} diff --git a/server/service/oauth.go b/server/service/oauth.go index bd30706..08130f6 100644 --- a/server/service/oauth.go +++ b/server/service/oauth.go @@ -68,16 +68,22 @@ type GithubUser struct { } // MakeAuthURL returns string for redirect to provider -func (o *OAuth) MakeAuthURL(w http.ResponseWriter, r *http.Request) string { +func (o *OAuth) MakeAuthURL(w http.ResponseWriter, r *http.Request) (string, error) { b := make([]byte, 16) rand.Read(b) state := base64.URLEncoding.EncodeToString(b) - session, _ := o.store.Get(r, "sess") + session, err := o.store.Get(r, "sess") + if err != nil { + return "", fmt.Errorf("could not save state under the current session: %s", err) + } + session.Values["state"] = state - session.Save(r, w) + if err := session.Save(r, w); err != nil { + return "", fmt.Errorf("could not save state under the current session: %s", err) + } - return o.config.AuthCodeURL(state) + return o.config.AuthCodeURL(state), nil } // ValidateState protects the user from CSRF attacks @@ -87,8 +93,9 @@ func (o *OAuth) ValidateState(r *http.Request, state string) error { return fmt.Errorf("can't get session: %s", err) } - if state != session.Values["state"] { - return fmt.Errorf("incorrect state: %s", state) + expectedState := session.Values["state"] + if state != expectedState { + return fmt.Errorf("incorrect state: %s; expected: %s", state, expectedState) } return nil diff --git a/src/App.js b/src/App.js index 63109b5..a757da6 100644 --- a/src/App.js +++ b/src/App.js @@ -4,6 +4,7 @@ import { Helmet } from 'react-helmet'; import { namedRoutes } from './state/routes'; import Errors from './components/Errors'; import Index from './pages/Index'; +import Auth from './pages/Auth'; import Experiments from './pages/Experiments'; import Experiment from './pages/Experiment'; import Final from './pages/Final'; @@ -21,6 +22,9 @@ class App extends Component { + + + diff --git a/src/api/index.js b/src/api/index.js index d7eb08b..c92bc0e 100644 --- a/src/api/index.js +++ b/src/api/index.js @@ -55,6 +55,7 @@ function apiCall(url, options = {}) { const token = TokenService.get(); const fetchOptions = { ...options, + credentials: 'include', headers: { ...options.headers, Authorization: `Bearer ${token}`, @@ -76,6 +77,10 @@ function apiCall(url, options = {}) { .catch(err => Promise.reject(normalizeErrors(err))); } +function auth(queryString) { + return apiCall(`/api/auth${queryString}`); +} + function me() { return apiCall(`/api/me`); } @@ -137,6 +142,7 @@ function exportDownload(filename) { } export default { + auth, me, getExperiments, getExperiment, diff --git a/src/pages/Auth.js b/src/pages/Auth.js new file mode 100644 index 0000000..99ce0e6 --- /dev/null +++ b/src/pages/Auth.js @@ -0,0 +1,34 @@ +import React, { Component } from 'react'; +import { connect } from 'react-redux'; +import { Grid, Row, Col } from 'react-bootstrap'; +import { Helmet } from 'react-helmet'; +import PageHeader from '../components/PageHeader'; +import Loader from '../components/Loader'; +import { auth } from '../state/user'; +import './Auth.less'; + +class Auth extends Component { + componentDidMount() { + this.props.auth(); + } + + render() { + return ( +
+ + Authorization + + + + + + + + + +
+ ); + } +} + +export default connect(undefined, { auth })(Auth); diff --git a/src/pages/Auth.less b/src/pages/Auth.less new file mode 100644 index 0000000..8d82c37 --- /dev/null +++ b/src/pages/Auth.less @@ -0,0 +1,6 @@ +.auth-page { + &__loader { + margin-top: 40px; + text-align: center; + } +} diff --git a/src/state/routes.js b/src/state/routes.js index 1bda879..d66b9d3 100644 --- a/src/state/routes.js +++ b/src/state/routes.js @@ -29,6 +29,10 @@ const routes = { name: 'question', }, }, + '/oauth-callback': { + name: 'auth', + public: true, + }, '/export': { name: 'export', restrictReviewer: true, diff --git a/src/state/routes.test.js b/src/state/routes.test.js index 24db16b..6a41406 100644 --- a/src/state/routes.test.js +++ b/src/state/routes.test.js @@ -24,7 +24,9 @@ describe('routers', () => { payload: {}, }; store.dispatch(action); - expect(store.getActions()).toEqual([]); + expect(store.getActions()).toEqual([ + { payload: {}, type: 'ROUTER_LOCATION_CHANGED' }, + ]); }); describe('experiment', () => { diff --git a/src/state/user.js b/src/state/user.js index 52900e7..9f69a64 100644 --- a/src/state/user.js +++ b/src/state/user.js @@ -57,36 +57,44 @@ export const logIn = () => dispatch => return dispatch(logOut()); }); +export const auth = () => (dispatch, getState) => { + const { router } = getState(); + return api + .auth(router.search) + .then(data => { + TokenService.set(data.token); + return dispatch(logIn()); + }) + .then(() => dispatch(push('/dashboard'))) + .catch(e => { + dispatch(addError(e)); + return dispatch(logOut()); + }); +}; + export const authMiddleware = store => next => action => { if (action.type !== LOCATION_CHANGED) { return next(action); } - const { query } = action.payload; - let promise = Promise.resolve(); - if (query && query.token) { - TokenService.set(query.token); - promise = next(logIn()); + + // redirect to / if try to access not public route wihout being logged in + const { user } = store.getState(); + const { result } = action.payload; + if (!user.loggedIn && result && !result.public) { + return next(replace('/')); } - return promise - .then(() => { - // redirect to / if try to access not public route wihout being logged in - const { user } = store.getState(); - const { result } = action.payload; - if (!user.loggedIn && result && !result.public) { - return next(replace('/')); - } - // redirect user from index page to experiment if user it authorized already - if (user.loggedIn && result && result.name === 'index') { - return next(push(makeUrl('dashboard'))); - } - // hide pages that are meant only for users with the requester role - if (user.role !== 'requester' && result && result.restrictReviewer) { - return next(replace('/forbidden')); - } - return next(action); - }) - .catch(e => next(addError(e))); + // redirect user from index page to experiment if user it authorized already + if (user.loggedIn && result && result.name === 'index') { + return next(push(makeUrl('dashboard'))); + } + + // hide pages that are meant only for users with the requester role + if (user.role !== 'requester' && result && result.restrictReviewer) { + return next(replace('/forbidden')); + } + + return next(action); }; export default reducer;