Merge pull request #35 from uc-cdis/fix/do-not-allow-infinite-access-…

…tokens

(PXP-8420): Do not allow infinite access tokens

.secrets.baseline

@@ -3,7 +3,7 @@
     "files": "^.secrets.baseline$",
     "lines": null
   },
-  "generated_at": "2021-04-26T04:50:04Z",
+  "generated_at": "2021-07-06T06:40:24Z",
   "plugins_used": [
     {
       "name": "AWSKeyDetector"
@@ -70,7 +70,7 @@
       {
         "hashed_secret": "b60d121b438a380c343d5ec3c2037564b82ffef3",
         "is_verified": false,
-        "line_number": 56,
+        "line_number": 60,
         "type": "Secret Keyword"
       }
     ],

README.md

@@ -15,7 +15,11 @@ OpenAPI Specification [here](http://petstore.swagger.io/?url=https://raw.githubu
 
 ## Authentication
 
-If a JWT access token is supplied as a header in the request to the `/token` endpoint, `WTS` validates the supplied token and returns a new access token for the user. Otherwise, at the moment, as displayed in the illustration below, if no authorization header is provided, the `/token` endpoint returns a new access token corresponding to the `gen3username` K8s annotation set for the requesting pod.
+Two methods exist for authenticating a request to the `/token` endpoint:
+
+1) `WTS` checks for the presence of a JWT access token in the `Authorization` header and validates said token. To prevent a user from being able to indefinitely generate access tokens using access tokens returned from `WTS`, this means of authentication is not available for requests with the `idp` URL query parameter omitted or `idp=default`.
+
+2) If a JWT access token is not provided, the `idp` parameter is omitted, or `idp=default`, then `WTS` determines the current user from the `gen3username` K8s annotation set for the requesting pod.
 
 <img src="docs/img/architecture.svg">
 

tests/app_test.py

@@ -102,20 +102,16 @@ def test_token_endpoint_with_default_idp(client, test_user, db_session, auth_hea
     logged_in_user_data = create_logged_in_user_data(test_user, db_session)
     create_other_user_data(db_session)
 
-    # the token returned for a specific IDP should be created using the
-    # corresponding refresh_token, using the logged in user's username
     res = client.get("/token/?idp=default", headers=auth_header)
-    assert res.status_code == 200
-    assert (
-        res.json["token"]
-        == "access_token_for_" + logged_in_user_data["default"]["refresh_token"]
-    )
+    assert res.status_code == 403
 
 
 def test_token_endpoint_with_idp_a(client, test_user, db_session, auth_header):
     logged_in_user_data = create_logged_in_user_data(test_user, db_session)
     create_other_user_data(db_session)
 
+    # the token returned for a specific IDP should be created using the
+    # corresponding refresh_token, using the logged in user's username
     res = client.get("/token/?idp=idp_a", headers=auth_header)
     assert res.status_code == 200
     assert (
@@ -130,13 +126,8 @@ def test_token_endpoint_without_specifying_idp(
     logged_in_user_data = create_logged_in_user_data(test_user, db_session)
     create_other_user_data(db_session)
 
-    # make sure the IDP we use is "default" when no IDP is requested
     res = client.get("/token/", headers=auth_header)
-    assert res.status_code == 200
-    assert (
-        res.json["token"]
-        == "access_token_for_" + logged_in_user_data["default"]["refresh_token"]
-    )
+    assert res.status_code == 403
 
 
 def test_token_endpoint_with_bogus_idp(client, test_user, db_session, auth_header):

wts/auth.py

@@ -1,16 +1,10 @@
-from functools import wraps
 import flask
+
 from .auth_plugins import find_user
 from cdiserrors import AuthError
 
 
-def login_required(f):
-    @wraps(f)
-    def decorated_function(*args, **kwargs):
-        if not hasattr(flask.g, "user"):
-            flask.g.user = find_user()
-            if not flask.g.user:
-                raise AuthError("You need to be authenticated to use this resource")
-        return f(*args, **kwargs)
-
-    return decorated_function
+def authenticate(allow_access_token=False):
+    flask.g.user = find_user(allow_access_token)
+    if not flask.g.user:
+        raise AuthError("You need to be authenticated to use this resource")

wts/auth_plugins/__init__.py

@@ -1,9 +1,9 @@
-from .base import DefaultPlugin
+from .base import AccessTokenPlugin
 from .k8s import K8SPlugin
 import flask
 
 
-def find_user():
-    if flask.request.headers.get("Authorization"):
-        return DefaultPlugin().find_user()
+def find_user(allow_access_token=False):
+    if allow_access_token and flask.request.headers.get("Authorization"):
+        return AccessTokenPlugin().find_user()
     return K8SPlugin().find_user()

wts/auth_plugins/base.py

@@ -11,7 +11,7 @@ def __init__(self, userid, username=None):
         self.username = username
 
 
-class DefaultPlugin(object):
+class AccessTokenPlugin(object):
     def __init__(self):
         pass
 

wts/blueprints/tokens.py

@@ -1,6 +1,6 @@
 import flask
 
-from ..auth import login_required
+from ..auth import authenticate
 from ..tokens import get_access_token
 
 
@@ -10,14 +10,15 @@
 
 
 @blueprint.route("/")
-@login_required
 def get_token():
+    requested_idp = flask.request.args.get("idp", "default")
+    authenticate(allow_access_token=(requested_idp != "default"))
+
     expires = flask.request.args.get("expires")
     if expires:
         try:
             expires = int(expires)
         except ValueError:
             return flask.jsonify({"error": "expires has to be an integer"}), 400
 
-    requested_idp = flask.request.args.get("idp", "default")
     return flask.jsonify({"token": get_access_token(requested_idp, expires=expires)})