fork/cellxgene
1
0
Fork 0
mirror of https://github.com/chanzuckerberg/cellxgene.git synced 2026-09-17 05:47:58 +08:00
Code Issues Packages Projects Releases Wiki Activity

Separate userinfo from the config endpoint (#1728)

Browse Source 
* Separate userinfo from the config endpoint

previously information about if the user was logged in and their username
was part of the config endpoint.
However, the config endpoint was previously static, and has a cache control.
Rather than not caching the config, a new endpoint called "userinfo"
is created to handle that information.

The config endpoint still has the non-changing part of the authentication:

  config:
    authentication:
        requires_client_login:  True/False
        login: <uri to login endoint if requires_client_login is True>
        logout: <uri to logout endoint if requires_client_login is True>

The userinfo endpoint returns this information:

  userinfo:
    is_authenticated:  True/False
    username: <string if is_authenticated>

if authentication is not enabled then the config does not have an authentication key,
and userinfo returns None.

Also in the PR are a few minor code improvements and bug fixes

Co-authored-by: Colin Megill <colinmegill@gmail.com>
This commit is contained in:
bmccandless
2020-08-17 13:41:03 -07:00
committed by GitHub
parent 4ad9f5875a
commit 298924fef5
12 changed files with 199 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
client/src/actions/index.js
View File

@@ -41,6 +41,17 @@ async function configFetch(dispatch) {
});
}
async function userInfoFetch(dispatch) {
return fetchJson("userinfo").then((response) => {
const userinfo = { ...response.userinfo };
dispatch({
type: "userinfo load complete",
userinfo,
});
return userinfo;
});
}
function prefetchEmbeddings(annoMatrix) {
/*
prefetch requests for all embeddings
@@ -62,6 +73,7 @@ const doInitialDataLoad = () =>
configFetch(dispatch),
schemaFetch(dispatch),
userColorsFetchAndLoad(dispatch),
userInfoFetch(dispatch),
]);
const baseDataUrl = `${globals.API.prefix}${globals.API.version}`;

10
client/src/components/autosave/filenameDialog.js
View File

@@ -14,6 +14,7 @@ import {
idhash: state.config?.parameters?.["annotations-user-data-idhash"] ?? null,
annotations: state.annotations,
auth: state.config?.authentication,
userinfo: state.userinfo,
writableCategoriesEnabled: state.config?.parameters?.annotations ?? false,
}))
class FilenameDialog extends React.Component {
@@ -91,13 +92,18 @@ class FilenameDialog extends React.Component {
};
render() {
const { writableCategoriesEnabled, annotations, idhash, auth } = this.props;
const {
writableCategoriesEnabled,
annotations,
idhash,
userinfo,
} = this.props;
const { filenameText } = this.state;
return writableCategoriesEnabled &&
!annotations.dataCollectionNameIsReadOnly &&
!annotations.dataCollectionName &&
auth.is_authenticated ? (
userinfo.is_authenticated ? (
<Dialog
icon="tag"
title="Annotations Collection"

33
client/src/components/categorical/index.js
View File

@@ -1,6 +1,6 @@
// jshint esversion: 6
import React from "react";
import { Button } from "@blueprintjs/core";
import { AnchorButton, Tooltip, Position } from "@blueprintjs/core";
import { connect } from "react-redux";
import * as globals from "../../globals";
import Category from "./category";
@@ -15,6 +15,7 @@ import actions from "../../actions";
writableCategoriesEnabled: state.config?.parameters?.annotations ?? false,
schema: state.annoMatrix?.schema,
ontology: state.ontology,
userinfo: state.userinfo,
}))
class Categories extends React.Component {
constructor(props) {
@@ -127,7 +128,12 @@ class Categories extends React.Component {
newCategoryText,
expandedCats,
} = this.state;
const { writableCategoriesEnabled, schema, ontology } = this.props;
const {
writableCategoriesEnabled,
schema,
ontology,
userinfo,
} = this.props;
const ontologyEnabled = ontology?.enabled ?? false;
/* all names, sorted in display order. Will be rendered in this order */
const allCategoryNames = ControlsHelpers.selectableCategoryNames(
@@ -203,15 +209,30 @@ class Categories extends React.Component {
)}
{writableCategoriesEnabled ? (
<div>
<Button
<Tooltip
content={
userinfo.is_authenticated
? "Create a new category"
: "You must be logged in to create new categorical fields"
}
position={Position.RIGHT}
boundary="viewport"
hoverOpenDelay={globals.tooltipHoverOpenDelay}
modifiers={{
preventOverflow: { enabled: false },
hide: { enabled: false },
}}
>
<AnchorButton
type="button"
data-testid="open-annotation-dialog"
onClick={this.handleEnableAnnoMode}
intent="primary"
disabled={!userinfo.is_authenticated}
>
Create new category
</Button>
</div>
</AnchorButton>
</Tooltip>
) : null}
</div>
);

8
client/src/components/menubar/authButtons.js
View File

@@ -4,7 +4,7 @@ import * as globals from "../../globals";
import styles from "./menubar.css";
const Auth = React.memo((props) => {
const { auth } = props;
const { auth, userinfo } = props;
if (!auth || (auth && !auth.requires_client_login)) return null;
@@ -19,10 +19,10 @@ const Auth = React.memo((props) => {
type="button"
data-testid="auth-button"
disabled={false}
icon={!auth.is_authenticated ? "log-in" : "log-out"}
href={!auth.is_authenticated ? auth.login : auth.logout}
icon={!userinfo["is_authenticated"] ? "log-in" : "log-out"}
href={!userinfo.is_authenticated ? auth.login : auth.logout}
>
{!auth.is_authenticated ? "Log In" : "Log Out"}
{!userinfo.is_authenticated ? "Log In" : "Log Out"}
</AnchorButton>
</Tooltip>
</div>

4
client/src/components/menubar/index.js
View File

@@ -42,6 +42,7 @@ import { getEmbSubsetView } from "../../util/stateManager/viewStackHelpers";
celllist2: state.differential.celllist2,
libraryVersions: state.config?.["library_versions"],
auth: state.config?.authentication,
userinfo: state.userinfo,
undoDisabled: state["@@undoable/past"].length === 0,
redoDisabled: state["@@undoable/future"].length === 0,
aboutLink: state.config?.links?.["about-dataset"],
@@ -221,6 +222,7 @@ class MenuBar extends React.PureComponent {
subsetResetPossible,
enableReembedding,
auth,
userinfo,
} = this.props;
const { pendingClipPercentiles } = this.state;
@@ -246,7 +248,7 @@ class MenuBar extends React.PureComponent {
zIndex: 3,
}}
>
<AuthButtons auth={auth} />
<AuthButtons auth={auth} userinfo={userinfo} />
<InformationMenu
libraryVersions={libraryVersions}
aboutLink={aboutLink}

2
client/src/reducers/index.js
View File

@@ -4,6 +4,7 @@ import thunk from "redux-thunk";
import cascadeReducers from "./cascade";
import undoable from "./undoable";
import config from "./config";
import userinfo from "./userinfo";
import annoMatrix from "./annoMatrix";
import obsCrossfilter from "./obsCrossfilter";
import categoricalSelection from "./categoricalSelection";
@@ -41,6 +42,7 @@ const Reducer = undoable(
["pointDilation", pointDialation],
["reembedController", reembedController],
["autosave", autosave],
["userinfo", userinfo],
]),
[
"annoMatrix",

27
client/src/reducers/userinfo.js Normal file
View File

@@ -0,0 +1,27 @@
// jshint esversion: 6
const UserInfo = (state = {}, action) => {
switch (action.type) {
case "initial data load start":
return {
...state,
loading: true,
error: null,
};
case "userinfo load complete":
return {
...state,
loading: false,
error: null,
...action.userinfo,
};
case "initial data load error":
return {
...state,
error: action.error,
};
default:
return state;
}
};
export default UserInfo;

8
server/app/app.py
View File

@@ -240,6 +240,13 @@ class ConfigAPI(DatasetResource):
return common_rest.config_get(current_app.app_config, data_adaptor)
class UserInfoAPI(DatasetResource):
@cache_control_always(no_store=True)
@rest_get_data_adaptor
def get(self, data_adaptor):
return common_rest.userinfo_get(current_app.app_config, data_adaptor)
class AnnotationsObsAPI(DatasetResource):
@cache_control(public=True, max_age=ONE_WEEK)
@rest_get_data_adaptor
@@ -308,6 +315,7 @@ def get_api_resources(bp_api, url_dataroot=None):
# Initialization routes
add_resource(SchemaAPI, "/schema")
add_resource(ConfigAPI, "/config")
add_resource(UserInfoAPI, "/userinfo")
# Data routes
add_resource(AnnotationsObsAPI, "/annotations/obs")
add_resource(AnnotationsVarAPI, "/annotations/var")

31
server/auth/auth_oauth.py
View File

@@ -1,4 +1,4 @@
from flask import session, request, redirect, current_app, after_this_request, has_request_context, g
from flask import session, request, redirect, current_app, has_request_context, g
from server.auth.auth import AuthTypeClientBase, AuthTypeFactory
from server.common.errors import AuthenticationError, ConfigurationError
from urllib.parse import urlencode
@@ -15,6 +15,7 @@ except ModuleNotFoundError:
try:
from jose import jwt
from jose.exceptions import ExpiredSignatureError, JWTError, JWTClaimsError
except ModuleNotFoundError:
missingimport.append("jose")
@@ -137,18 +138,15 @@ class AuthTypeOAuth(AuthTypeClientBase):
return response
def logout(self):
params = {'returnTo' : self.callback_base_url, 'client_id' : self.client_id}
response = redirect(self.client.api_base_url + '/v2/logout?' + urlencode(params))
if self.session_cookie:
if self.CXG_ID_TOKEN in session:
del session[self.CXG_ID_TOKEN]
else:
@after_this_request
def remove_cookie(response):
response.set_cookie(self.cookie_params["key"], "", expires=0)
self.update_response(response)
return response
response.set_cookie(self.cookie_params["key"], "", expires=0)
params = {'returnTo' : self.callback_base_url, 'client_id' : self.client_id}
response = redirect(self.client.api_base_url + '/v2/logout?' + urlencode(params))
self.update_response(response)
return response
@@ -178,7 +176,7 @@ class AuthTypeOAuth(AuthTypeClientBase):
def get_login_url(self, data_adaptor):
"""Return the url for the login route"""
if current_app.app_config.is_multi_dataset():
return f"/login?dataset={data_adaptor.uri_path}"
return f"/login?dataset={data_adaptor.uri_path}/"
else:
return "/login"
@@ -205,7 +203,11 @@ class AuthTypeOAuth(AuthTypeClientBase):
if token is None:
return None
unverified_header = jwt.get_unverified_header(token)
try:
unverified_header = jwt.get_unverified_header(token)
except JWTError:
return None
rsa_key = {}
for key in self.jwks['keys']:
if key['kid'] == unverified_header['kid']:
@@ -227,11 +229,12 @@ class AuthTypeOAuth(AuthTypeClientBase):
)
return payload
except jwt.JWTError as e:
except JWTError as e:
raise AuthenticationError(f"invalid signature: {str(e)}")
except jwt.ExpiredSignatureError as e:
raise AuthenticationError(f"token expired: {str(e)}")
except jwt.JWTClaimsError as e:
except ExpiredSignatureError:
# TODO, handle expired sessions by refreshing the token
return None
except JWTClaimsError as e:
raise AuthenticationError(f"invalid claims {str(e)}")
raise AuthenticationError("Unable to find the appropriate key")

26
server/common/app_config.py
View File

@@ -275,10 +275,7 @@ class AppConfig(object):
if dataset_config.app__authentication_enable and auth.is_valid_authentication_type():
config["authentication"] = {
"is_authenticated": auth.is_user_authenticated(),
"requires_client_login": auth.requires_client_login(),
"username": auth.get_user_name(),
"user_id": auth.get_user_id()
}
if auth.requires_client_login():
config["authentication"].update({
@@ -288,6 +285,29 @@ class AppConfig(object):
return c
def get_client_userinfo(self, data_adaptor):
"""
Return the userinfo as required by the /userinfo REST route
"""
server_config = self.server_config
dataset_config = data_adaptor.dataset_config
auth = server_config.auth
# make sure the configuration has been checked.
self.check_config()
if dataset_config.app__authentication_enable and auth.is_valid_authentication_type():
userinfo = {}
userinfo["userinfo"] = {
"is_authenticated": auth.is_user_authenticated(),
"username": auth.get_user_name(),
"user_id": auth.get_user_id()
}
return userinfo
else:
return None
class BaseConfig(object):
"""This class handles the mechanics of updating and checking attributes.

5
server/common/rest.py
View File

@@ -121,6 +121,11 @@ def config_get(app_config, data_adaptor):
return make_response(jsonify(config), HTTPStatus.OK)
def userinfo_get(app_config, data_adaptor):
config = app_config.get_client_userinfo(data_adaptor)
return make_response(jsonify(config), HTTPStatus.OK)
def annotations_obs_get(request, data_adaptor):
fields = request.args.getlist("annotation-name", None)
num_columns_requested = len(data_adaptor.get_obs_keys()) if len(fields) == 0 else len(fields)

124
server/test/unit/auth/test_auth.py
View File

@@ -21,9 +21,10 @@ class AuthTest(unittest.TestCase):
with test_server(app_config=c) as server:
session = requests.Session()
r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
data_config = r.json()
assert "authentication" not in data_config["config"]
config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json()
userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json()
self.assertNotIn("authentication", config["config"])
self.assertIsNone(userinfo)
def test_auth_session(self):
c = AppConfig()
@@ -35,11 +36,12 @@ class AuthTest(unittest.TestCase):
with test_server(app_config=c) as server:
session = requests.Session()
r = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config")
data_config = r.json()
assert data_config["config"]["authentication"]["is_authenticated"]
assert not data_config["config"]["authentication"]["requires_client_login"]
assert data_config["config"]["authentication"]["username"] == "anonymous"
config = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/config").json()
userinfo = session.get(f"{server}/d/pbmc3k.cxg/api/v0.2/userinfo").json()
self.assertFalse(config["config"]["authentication"]["requires_client_login"])
self.assertTrue(userinfo["userinfo"]["is_authenticated"])
self.assertEqual(userinfo["userinfo"]["username"], "anonymous")
def test_auth_test(self):
c = AppConfig()
@@ -61,45 +63,46 @@ class AuthTest(unittest.TestCase):
session = requests.Session()
# auth datasets
r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config")
data_config = r.json()
assert not data_config["config"]["authentication"]["is_authenticated"]
assert data_config["config"]["authentication"]["requires_client_login"]
assert data_config["config"]["authentication"]["username"] is None
assert data_config["config"]["parameters"]["annotations"]
config = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config").json()
userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
login_uri = data_config["config"]["authentication"]["login"]
logout_uri = data_config["config"]["authentication"]["logout"]
self.assertFalse(userinfo["userinfo"]["is_authenticated"])
self.assertIsNone(userinfo["userinfo"]["username"])
self.assertTrue(config["config"]["authentication"]["requires_client_login"])
self.assertTrue(config["config"]["parameters"]["annotations"])
assert login_uri == "/login?dataset=auth/pbmc3k.cxg"
assert logout_uri == "/logout?dataset=auth/pbmc3k.cxg"
login_uri = config["config"]["authentication"]["login"]
logout_uri = config["config"]["authentication"]["logout"]
self.assertEqual(login_uri, "/login?dataset=auth/pbmc3k.cxg")
self.assertEqual(logout_uri, "/logout?dataset=auth/pbmc3k.cxg")
r = session.get(f"{server}/{login_uri}")
# check that the login redirect worked
assert r.history[0].status_code == 302
assert r.url == f"{server}/auth/pbmc3k.cxg/"
self.assertEqual(r.history[0].status_code, 302)
self.assertEqual(r.url, f"{server}/auth/pbmc3k.cxg/")
r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config")
data_config = r.json()
assert data_config["config"]["authentication"]["is_authenticated"]
assert data_config["config"]["authentication"]["username"] == "test_account"
assert data_config["config"]["parameters"]["annotations"]
config = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config").json()
userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
self.assertTrue(userinfo["userinfo"]["is_authenticated"])
self.assertEqual(userinfo["userinfo"]["username"], "test_account")
self.assertTrue(config["config"]["parameters"]["annotations"])
r = session.get(f"{server}/{logout_uri}")
# check that the logout redirect worked
assert r.history[0].status_code == 302
assert r.url == f"{server}/auth/pbmc3k.cxg/"
r = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config")
data_config = r.json()
assert not data_config["config"]["authentication"]["is_authenticated"]
assert data_config["config"]["authentication"]["username"] is None
assert data_config["config"]["parameters"]["annotations"]
self.assertEqual(r.history[0].status_code, 302)
self.assertEqual(r.url, f"{server}/auth/pbmc3k.cxg/")
config = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/config").json()
userinfo = session.get(f"{server}/auth/pbmc3k.cxg/api/v0.2/userinfo").json()
self.assertFalse(userinfo["userinfo"]["is_authenticated"])
self.assertIsNone(userinfo["userinfo"]["username"])
self.assertTrue(config["config"]["parameters"]["annotations"])
# no-auth datasets
r = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/config")
data_config = r.json()
assert "authentication" not in data_config["config"]
assert not data_config["config"]["parameters"]["annotations"]
config = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/config").json()
userinfo = session.get(f"{server}/no-auth/pbmc3k.cxg/api/v0.2/userinfo").json()
self.assertIsNone(userinfo)
self.assertFalse(config["config"]["parameters"]["annotations"])
def test_auth_test_single(self):
c = AppConfig()
@@ -111,37 +114,36 @@ class AuthTest(unittest.TestCase):
with test_server(app_config=c) as server:
session = requests.Session()
config = session.get(f"{server}/api/v0.2/config").json()
userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
self.assertFalse(userinfo["userinfo"]["is_authenticated"])
self.assertIsNone(userinfo["userinfo"]["username"])
self.assertTrue(config["config"]["authentication"]["requires_client_login"])
self.assertTrue(config["config"]["parameters"]["annotations"])
r = session.get(f"{server}/api/v0.2/config")
data_config = r.json()
assert not data_config["config"]["authentication"]["is_authenticated"]
assert data_config["config"]["authentication"]["requires_client_login"]
assert data_config["config"]["authentication"]["username"] is None
assert data_config["config"]["parameters"]["annotations"]
login_uri = config["config"]["authentication"]["login"]
logout_uri = config["config"]["authentication"]["logout"]
login_uri = data_config["config"]["authentication"]["login"]
logout_uri = data_config["config"]["authentication"]["logout"]
assert login_uri == "/login"
assert logout_uri == "/logout"
self.assertEqual(login_uri, "/login")
self.assertEqual(logout_uri, "/logout")
r = session.get(f"{server}/{login_uri}")
# check that the login redirect worked
assert r.history[0].status_code == 302
assert r.url == f"{server}/"
self.assertEqual(r.history[0].status_code, 302)
self.assertEqual(r.url, f"{server}/")
r = session.get(f"{server}/api/v0.2/config")
data_config = r.json()
assert data_config["config"]["authentication"]["is_authenticated"]
assert data_config["config"]["authentication"]["username"] == "test_account"
assert data_config["config"]["parameters"]["annotations"]
config = session.get(f"{server}/api/v0.2/config").json()
userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
self.assertTrue(userinfo["userinfo"]["is_authenticated"])
self.assertEqual(userinfo["userinfo"]["username"], "test_account")
self.assertTrue(config["config"]["parameters"]["annotations"])
r = session.get(f"{server}/{logout_uri}")
# check that the logout redirect worked
assert r.history[0].status_code == 302
assert r.url == f"{server}/"
r = session.get(f"{server}/api/v0.2/config")
data_config = r.json()
assert not data_config["config"]["authentication"]["is_authenticated"]
assert data_config["config"]["authentication"]["username"] is None
assert data_config["config"]["parameters"]["annotations"]
self.assertEqual(r.history[0].status_code, 302)
self.assertEqual(r.url, f"{server}/")
config = session.get(f"{server}/api/v0.2/config").json()
userinfo = session.get(f"{server}/api/v0.2/userinfo").json()
self.assertFalse(userinfo["userinfo"]["is_authenticated"])
self.assertIsNone(userinfo["userinfo"]["username"])
self.assertTrue(config["config"]["parameters"]["annotations"])