Handle auth in front end

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:

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` | | `<CAT_HOST>:<CAT_PORT>` | URL used to access the application (i.e. public hostname) |
-| `CAT_UI_DOMAIN` | | `<CAT_HOST>:<CAT_PORT>` | 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://<your-hostname>/export` |
 | `CAT_ENV` | | `production` | Sets the log level. Use `dev` to enable debug log messages |

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)

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

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,29 +83,23 @@ 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
 			user.AvatarURL = ghUser.AvatarURL
 			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
 	}
 }

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

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)

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})
+}

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

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 {
           <Fragment forRoute={namedRoutes.index}>
             <Index />
           </Fragment>
+          <Fragment forRoute={namedRoutes.auth}>
+            <Auth />
+          </Fragment>
           <Fragment forRoute={namedRoutes.dashboard}>
             <Experiments />
           </Fragment>

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,

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 (
+      <div className="auth-page">
+        <Helmet>
+          <title>Authorization</title>
+        </Helmet>
+        <PageHeader />
+        <Grid>
+          <Row>
+            <Col xs={12} className="auth-page__loader">
+              <Loader />
+            </Col>
+          </Row>
+        </Grid>
+      </div>
+    );
+  }
+}
+
+export default connect(undefined, { auth })(Auth);

src/pages/Auth.less

@@ -0,0 +1,6 @@
+.auth-page {
+    &__loader {
+        margin-top: 40px;
+        text-align: center;
+    }
+}

src/state/routes.js

@@ -29,6 +29,10 @@ const routes = {
         name: 'question',
       },
     },
+    '/oauth-callback': {
+      name: 'auth',
+      public: true,
+    },
     '/export': {
       name: 'export',
       restrictReviewer: true,

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', () => {