Fixed bugs and refactored authentication in GUI and API.

.vscode/launch.json

@@ -44,7 +44,7 @@
       "cwd": "${workspaceFolder}",
       "program": "${workspaceFolder}/.pyenv/bin/pytest",
       "args": [
-        "-sv", "tests/test_migration.py::test_migrate"
+        "-sv", "tests/test_api.py::TestAuth::test_xtoken_auth_denied"
       ]
     },
     {

gui/src/components/LoginLogout.js

@@ -11,6 +11,7 @@ class LoginLogout extends React.Component {
   static propTypes = {
     classes: PropTypes.object.isRequired,
     api: PropTypes.object.isRequired,
+    isLoggingIn: PropTypes.bool,
     user: PropTypes.object,
     login: PropTypes.func.isRequired,
     logout: PropTypes.func.isRequired,
@@ -27,6 +28,7 @@ class LoginLogout extends React.Component {
       }
     },
     button: {}, // to allow overrides
+    buttonDisabled: {},
     errorText: {
       marginTop: theme.spacing.unit,
       marginBottom: theme.spacing.unit
@@ -43,7 +45,6 @@ class LoginLogout extends React.Component {
     loginDialogOpen: false,
     userName: '',
     password: '',
-    loggingIn: false,
     failure: false
   }
 
@@ -56,23 +57,19 @@ class LoginLogout extends React.Component {
   }
 
   handleLoginDialogClosed(withLogin) {
-    this.setState({loginDialogOpen: false})
     if (withLogin) {
-      if (this._ismounted) {
-        this.setState({loggingIn: true})
-      }
       this.props.login(this.state.userName, this.state.password, (success) => {
         if (this._ismounted) {
           if (success) {
-            this.setState({loggingIn: false, loginDialogOpen: false, failure: false})
+            this.setState({loginDialogOpen: false, failure: false})
           } else {
-            this.setState({loggingIn: false, failure: true, loginDialogOpen: true})
+            this.setState({failure: true, loginDialogOpen: true})
           }
         }
       })
     } else {
       if (this._ismounted) {
-        this.setState({loggingIn: false, failure: false, userName: '', password: '', loginDialogOpen: false})
+        this.setState({failure: false, userName: '', password: '', loginDialogOpen: false})
       }
     }
   }
@@ -88,8 +85,8 @@ class LoginLogout extends React.Component {
   }
 
   render() {
-    const { classes, user, variant, color } = this.props
-    const { loggingIn, failure } = this.state
+    const { classes, user, variant, color, isLoggingIn } = this.props
+    const { failure } = this.state
     if (user) {
       return (
         <div className={classes.root}>
@@ -107,7 +104,7 @@ class LoginLogout extends React.Component {
       return (
         <div className={classes.root}>
           <Button
-            className={classes.button} variant={variant} color={color}
+            className={isLoggingIn ? classes.buttonDisabled : classes.button} variant={variant} color={color} disabled={isLoggingIn}
             onClick={() => this.setState({loginDialogOpen: true})}
           >Login</Button>
           <Dialog
@@ -121,11 +118,11 @@ class LoginLogout extends React.Component {
                 do not have an account, please go to the nomad repository and
                 create one.
               </DialogContentText>
-              {loggingIn ? <LinearProgress/> : ''}
+              {isLoggingIn ? <LinearProgress/> : ''}
               {failure ? <DialogContentText className={classes.errorText} color="error">Wrong username or password!</DialogContentText> : ''}
               <FormGroup>
                 <TextField
-                  disabled={loggingIn}
+                  disabled={isLoggingIn}
                   autoFocus
                   margin="dense"
                   id="uaseName"
@@ -136,7 +133,7 @@ class LoginLogout extends React.Component {
                   onChange={this.handleChange('userName')}
                 />
                 <TextField
-                  disabled={loggingIn}
+                  disabled={isLoggingIn}
                   margin="dense"
                   id="password"
                   label="Password"

gui/src/components/Navigation.js

@@ -166,6 +166,9 @@ class Navigation extends React.Component {
     barButton: {
       borderColor: theme.palette.getContrastText(theme.palette.primary.main),
       marginRight: theme.spacing.unit * 4
+    },
+    barButtonDisabled: {
+      marginRight: theme.spacing.unit * 4
     }
   })
 
@@ -311,7 +314,7 @@ class Navigation extends React.Component {
                   {selected(toolbarTitles)}
                 </Typography>
                 <div className={classes.barActions}>
-                  <LoginLogout variant="outlined" color="inherit" classes={{button: classes.barButton}} />
+                  <LoginLogout variant="outlined" color="inherit" classes={{button: classes.barButton, buttonDisabled: classes.barButtonDisabled}} />
                   <FormLabel className={classes.barSelect} >Show help</FormLabel>
                   <HelpContext.Consumer>{
                     help => (

gui/src/components/api.js

 import React from 'react'
-import PropTypes from 'prop-types'
+import PropTypes, { instanceOf } from 'prop-types'
 import { withErrors } from './errors'
 import { UploadRequest } from '@navjobs/upload'
 import Swagger from 'swagger-client'
 import { apiBase } from '../config'
-import { Typography, withStyles } from '@material-ui/core'
+import { Typography, withStyles, LinearProgress } from '@material-ui/core'
 import LoginLogout from './LoginLogout'
+import { Cookies, withCookies } from 'react-cookie'
 
 const ApiContext = React.createContext()
 
@@ -282,19 +283,30 @@ class Api {
   }
 }
 
-export class ApiProvider extends React.Component {
+export class ApiProviderComponent extends React.Component {
   static propTypes = {
     children: PropTypes.oneOfType([
       PropTypes.arrayOf(PropTypes.node),
       PropTypes.node
-    ]).isRequired
+    ]).isRequired,
+    cookies: instanceOf(Cookies).isRequired
+  }
+
+  componentDidMount() {
+    const token = this.props.cookies.get('token')
+    if (token) {
+      this.state.login(token)
+    }
   }
 
   state = {
     api: new Api(),
     user: null,
-    login: (userName, password, successCallback) => {
-      Api.createSwaggerClient(userName, password)
+    isLoggingIn: false,
+    login: (userNameToken, password, successCallback) => {
+      this.setState({isLoggingIn: true})
+      successCallback = successCallback || (() => true)
+      Api.createSwaggerClient(userNameToken, password)
         .catch(this.state.api.handleApiError)
         .then(client => {
           client.apis.auth.get_user()
@@ -302,20 +314,24 @@ export class ApiProvider extends React.Component {
               if (error.response.status !== 401) {
                 this.handleApiError(error)
               }
+              this.setState({isLoggingIn: false})
             })
             .then(response => {
               if (response) {
                 const user = response.body
                 this.setState({api: new Api(user), user: user})
+                this.props.cookies.set('token', user.token)
                 successCallback(true)
               } else {
                 successCallback(false)
               }
+              this.setState({isLoggingIn: false})
             })
         })
     },
     logout: () => {
       this.setState({api: new Api(), user: null})
+      this.props.cookies.set('token', undefined)
     }
   }
 
@@ -331,7 +347,8 @@ export class ApiProvider extends React.Component {
 
 class LoginRequiredUnstyled extends React.Component {
   static propTypes = {
-    classes: PropTypes.object.isRequired
+    classes: PropTypes.object.isRequired,
+    isLoggingIn: PropTypes.bool
   }
 
   static styles = theme => ({
@@ -345,18 +362,24 @@ class LoginRequiredUnstyled extends React.Component {
   })
 
   render() {
-    const {classes} = this.props
-    return (
-      <div className={classes.root}>
-        <Typography>
-          To upload data, you must have a nomad account and you must be logged in.
-        </Typography>
-        <LoginLogout variant="outlined" color="primary"/>
-      </div>
-    )
+    const {classes, isLoggingIn} = this.props
+    if (!isLoggingIn) {
+      return (
+        <div className={classes.root}>
+          <Typography>
+            To upload data, you must have a nomad account and you must be logged in.
+          </Typography>
+          <LoginLogout variant="outlined" color="primary"/>
+        </div>
+      )
+    } else {
+      return <LinearProgress />
+    }
   }
 }
 
+export const ApiProvider = withCookies(ApiProviderComponent)
+
 const LoginRequired = withStyles(LoginRequiredUnstyled.styles)(LoginRequiredUnstyled)
 
 export function withApi(loginRequired) {
@@ -367,9 +390,8 @@ export function withApi(loginRequired) {
           {apiContext => (
             (apiContext.user || !loginRequired)
               ? <Component
-                {...props} api={apiContext.api} user={apiContext.user}
-                login={apiContext.login} logout={apiContext.logout} />
-              : <LoginRequired />
+                {...props} {...apiContext} />
+              : <LoginRequired {...apiContext} />
           )}
         </ApiContext.Consumer>
       )

nomad/api/auth.py

@@ -63,20 +63,26 @@ api.authorizations = {
 
 @auth.verify_password
 def verify_password(username_or_token, password):
-    # first try to authenticate by token
-    g.user = User.verify_auth_token(username_or_token)
-    if not g.user:
-        # try to authenticate with username/password
+    if username_or_token is None or username_or_token == '':
+        g.user = None
+        return True
+
+    if password is None or password == '':
+        g.user = User.verify_auth_token(username_or_token)
+        return g.user is not None
+    else:
         try:
             g.user = User.verify_user_password(username_or_token, password)
         except Exception as e:
             utils.get_logger(__name__).error('could not verify password', exc_info=e)
             return False
 
-    if not g.user:
-        return True  # anonymous access
+        return g.user is not None
 
-    return True
+
+@auth.error_handler
+def auth_error_handler():
+    abort(401, 'Could not authenticate user, bad credentials')
 
 
 def login_if_available(func):

nomad/api/upload.py

@@ -293,7 +293,7 @@ class UploadResource(Resource):
         try:
             upload.delete_upload()
         except ProcessAlreadyRunning:
-            abort(400, message='The upload is still/already processed')
+            abort(400, message='The upload is still processed')
         except Exception as e:
             logger.error('could not delete processing upload', exc_info=e)
             raise e

nomad/client/upload.py

@@ -60,7 +60,7 @@ def upload_file(file_path: str, name: str = None, offline: bool = False, commit:
             'status: %s; task: %s; parsing: %d/%d/%d                %s' %
             (upload.tasks_status, upload.current_task, successes, failures, total, ret), end='')
 
-        time.sleep(3)
+        time.sleep(1)
 
     if upload.tasks_status == FAILURE:
         click.echo('There have been errors:')

nomad/coe_repo/user.py

@@ -84,6 +84,9 @@ class User(Base):  # type: ignore
 
     @staticmethod
     def verify_user_password(email, password):
+        if email is None or password is None or email == '' or password == '':
+            return None
+
         repo_db = infrastructure.repository_db
         user = repo_db.query(User).filter_by(email=email).first()
         if not user:
@@ -96,6 +99,9 @@ class User(Base):  # type: ignore
 
     @staticmethod
     def verify_auth_token(token):
+        if token is None or token == '':
+            return None
+
         repo_db = infrastructure.repository_db
         session = repo_db.query(Session).filter_by(token=token).first()
         if session is None:
@@ -119,7 +125,7 @@ def ensure_test_user(email):
     session = repo_db.query(Session).filter_by(
         user_id=existing.user_id).first()
     assert session, 'Test user %s has no session.' % email
-    assert session.token == email, 'Test user %s session has unexpected token.' % email
+    assert session.token == existing.first_name.lower(), 'Test user %s session has unexpected token.' % email
 
     return existing
 

nomad/empty_repository_db.sql

@@ -1155,8 +1155,8 @@ INSERT INTO public.pragma VALUES ('4.59');
 -- Data for Name: sessions; Type: TABLE DATA; Schema: public; Owner: postgres
 --
 
-INSERT INTO public.sessions VALUES ('leonard.hofstadter@nomad-fairdi.tests.de', 3, '2100-12-17 09:00:00+00', NULL, NULL);
-INSERT INTO public.sessions VALUES ('sheldon.cooper@nomad-fairdi.tests.de', 2, '2100-12-17 09:00:00+00', NULL, NULL);
+INSERT INTO public.sessions VALUES ('leonard', 3, '2100-12-17 09:00:00+00', NULL, NULL);
+INSERT INTO public.sessions VALUES ('sheldon', 2, '2100-12-17 09:00:00+00', NULL, NULL);
 
 
 --

tests/test_api.py

@@ -123,12 +123,12 @@ class TestAdmin:
 class TestAuth:
     def test_xtoken_auth(self, client, test_user: User, no_warn):
         rv = client.get('/uploads/', headers={
-            'X-Token': test_user.email  # the test users have their email as tokens for convinience
+            'X-Token': test_user.first_name.lower()  # the test users have their firstname as tokens for convinience
         })
 
         assert rv.status_code == 200
 
-    def test_xtoken_auth_denied(self, client, no_warn):
+    def test_xtoken_auth_denied(self, client, no_warn, repository_db):
         rv = client.get('/uploads/', headers={
             'X-Token': 'invalid'
         })
@@ -301,6 +301,7 @@ class TestUploads:
             upload = self.assert_upload(rv.data, local_path=file, name=name)
         else:
             upload = self.assert_upload(rv.data, name=name)
+        assert upload['tasks_running']
 
         self.assert_processing(client, test_user_auth, upload['upload_id'])
 
@@ -308,9 +309,22 @@ class TestUploads:
         rv = client.delete('/uploads/123456789012123456789012', headers=test_user_auth)
         assert rv.status_code == 404
 
-    def test_delete_during_processing(self, client, test_user_auth, proc_infra):
+    @pytest.fixture(scope='function')
+    def slow_processing(self, monkeypatch):
+        old_cleanup = Upload.cleanup
+
+        def slow_cleanup(self):
+            time.sleep(0.5)
+            old_cleanup(self)
+
+        monkeypatch.setattr('nomad.processing.data.Upload.cleanup', slow_cleanup)
+        yield True
+        monkeypatch.setattr('nomad.processing.data.Upload.cleanup', old_cleanup)
+
+    def test_delete_during_processing(self, client, test_user_auth, proc_infra, slow_processing):
         rv = client.put('/uploads/?local_path=%s' % example_file, headers=test_user_auth)
         upload = self.assert_upload(rv.data)
+        assert upload['tasks_running']
         rv = client.delete('/uploads/%s' % upload['upload_id'], headers=test_user_auth)
         assert rv.status_code == 400
         self.assert_processing(client, test_user_auth, upload['upload_id'])

tests/test_coe_repo.py

@@ -30,7 +30,7 @@ def assert_user(user, reference):
 
 
 def test_token_authorize(test_user):
-    user = User.verify_auth_token(test_user.email)
+    user = User.verify_auth_token(test_user.first_name.lower())
     assert_user(user, test_user)