Skip to content

Commit

Permalink
Merge pull request #1106 from uc-cdis/feat/DCF-1446-mfa-arborist-poli…
Browse files Browse the repository at this point in the history
…cy-assignment

feat(DCF-1446): Assign MFA policy on login
  • Loading branch information
k-burt-uch authored Aug 25, 2023
2 parents 190d808 + 8c0513c commit ea885f0
Show file tree
Hide file tree
Showing 29 changed files with 562 additions and 108 deletions.
20 changes: 19 additions & 1 deletion .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,15 @@
"line_number": 7
}
],
"docs/fence_multifactor_authentication_guide.md": [
{
"type": "Secret Keyword",
"filename": "docs/fence_multifactor_authentication_guide.md",
"hashed_secret": "0f674908b6342fcf2a9842d04699cb008d57d399",
"is_verified": false,
"line_number": 38
}
],
"fence/blueprints/storage_creds/google.py": [
{
"type": "Private Key",
Expand Down Expand Up @@ -315,6 +324,15 @@
"line_number": 48
}
],
"tests/login/test_idp_oauth2.py": [
{
"type": "Secret Keyword",
"filename": "tests/login/test_idp_oauth2.py",
"hashed_secret": "f3bbbd66a63d4bf1747940578ec3d0103530e21d",
"is_verified": false,
"line_number": 8
}
],
"tests/migrations/test_a04a70296688.py": [
{
"type": "Hex High Entropy String",
Expand Down Expand Up @@ -368,5 +386,5 @@
}
]
},
"generated_at": "2023-05-15T16:30:09Z"
"generated_at": "2023-08-08T00:23:28Z"
}
22 changes: 20 additions & 2 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,14 +1,32 @@
language: python

dist: jammy
python:
- "3.9"

sudo: false

cache: pip

services:
- postgresql

addons:
postgresql: "9.6"
postgresql: "13"
apt:
sources:
- sourceline: deb http://apt.postgresql.org/pub/repos/apt/ jammy-pgdg main
13
key_url: https://www.postgresql.org/media/keys/ACCC4CF8.asc
packages:
- postgresql-13

before_install:
# Copy custom configs from the repo because PG-13 isn't set up to run like
# it normally does on Travis out of the box.
# Source: https://github.com/NCI-GDC/psqlgraph/blob/94f315db2c039217752cba85d9c63988f2059317/.travis.yml
- sudo cp travis/postgresql.conf /etc/postgresql/13/main/postgresql.conf
- sudo cp travis/pg_hba.conf /etc/postgresql/13/main/pg_hba.conf
- sudo pg_ctlcluster 13 main restart

install:
- pip install --upgrade pip
Expand Down
64 changes: 64 additions & 0 deletions docs/fence_multifactor_authentication_guide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# Fence Multifactor Authentication Guide

Fence is capable of using token claims from IdPs to identify when multifactor authentication (MFA) was used during the authentication process.

## File Level Enforcement
To restrict access to files to user who've authenticated with MFA, the following resource *MUST* be present in the indexd record's `authz`:
`/multifactor_auth`

And the following configs must be updated:
- fence-config.yaml
- user.yaml

### fence-config.yaml changes

MFA claim checking is configured on a per-IdP basis. For a given IdP, define the name of the claim in the id_token and is possible values that indicate MFA. If the id_token claim value matches at least one value in the configured multifactor_auth_claim_info.values, then "/multifactor_auth" resource will be assigned to the user.

For example, Okta may issue the following id_token when MFA is used:
```
{
"amr": ["otp", "pwd"],
"aud": "6joRGIzNCaJfdCPzRjlh",
"auth_time": 1311280970,
"exp": 1311280970,
"iat": 1311280970,
"idp": "00ok1u7AsAkrwdZL3z0g3",
"iss": "https://$"
"jti": "Tlenfse93dgkaksginv",
"sub": "00uk1u7AsAk6dZL3z0g3",
"ver": 1
}
```

And fence-config.yaml is configured as follows:
```
OPENID_CONNECT:
okta:
client_id: 'redacted'
client_secret: 'redacted'
multifactor_auth_claim_info:
claim: 'amr'
values: [ "mfa", "otp", "sms" ]
```

Then fence will assign the "/multifactor_auth" resource to the user in Arborist.

### user.yaml changes
The `mfa_policy` policy and `multifactor_auth` resource must be added to user.yaml so the appropriate policy and resource are created in arborist when usersync runs.

NOTE: The role_ids provided here are an example and should be changed to the appropriate arborist roles for the commons.

Add the following to the `resources` section:
```yaml
- name: multifactor_auth
```
Add the following the `policies` section:
```yaml
- id: mfa_policy
role_ids:
- read-storage
- read
resource_paths:
- /multifactor_auth
```
2 changes: 1 addition & 1 deletion fence/blueprints/link.py
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,7 @@ def get(self):
code = flask.request.args.get("code")

if not config.get("MOCK_GOOGLE_AUTH", False):
google_response = flask.current_app.google_client.get_user_id(code)
google_response = flask.current_app.google_client.get_auth_info(code)
email = google_response.get("email")
else:
# if we're mocking google auth, mock response to include the email
Expand Down
26 changes: 25 additions & 1 deletion fence/blueprints/login/base.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import flask
from cdislogging import get_logger
from flask_restful import Resource
from urllib.parse import urlparse, urlencode, parse_qsl

Expand All @@ -7,6 +8,8 @@
from fence.config import config
from fence.errors import UserError

logger = get_logger(__name__)


class DefaultOAuth2Login(Resource):
def __init__(self, idp_name, client):
Expand Down Expand Up @@ -63,6 +66,7 @@ def __init__(
username_field="email",
email_field="email",
id_from_idp_field="sub",
app=flask.current_app,
):
"""
Construct a resource for a login callback endpoint
Expand All @@ -84,6 +88,10 @@ def __init__(
self.username_field = username_field
self.email_field = email_field
self.id_from_idp_field = id_from_idp_field
self.is_mfa_enabled = "multifactor_auth_claim_info" in config[
"OPENID_CONNECT"
].get(self.idp_name, {})
self.app = app

def get(self):
# Check if user granted access
Expand All @@ -109,7 +117,7 @@ def get(self):
return flask.redirect(location=final_redirect_url)

code = flask.request.args.get("code")
result = self.client.get_user_id(code)
result = self.client.get_auth_info(code)
username = result.get(self.username_field)
if not username:
raise UserError(
Expand All @@ -125,6 +133,22 @@ def get(self):

def post_login(self, user=None, token_result=None, **kwargs):
prepare_login_log(self.idp_name)
if token_result:
username = token_result.get(self.username_field)
if self.is_mfa_enabled:
if token_result.get("mfa"):
logger.info(f"Adding mfa_policy for {username}")
self.app.arborist.grant_user_policy(
username=username,
policy_id="mfa_policy",
)
return
else:
logger.info(f"Revoking mfa_policy for {username}")
self.app.arborist.revoke_user_policy(
username=username,
policy_id="mfa_policy",
)


def prepare_login_log(idp_name):
Expand Down
2 changes: 1 addition & 1 deletion fence/blueprints/login/ras.py
Original file line number Diff line number Diff line change
Expand Up @@ -114,4 +114,4 @@ def post_login(self, user=None, token_result=None, id_from_idp=None):
user=user, refresh_token=refresh_token, expires=expires + issued_time
)

super(RASCallback, self).post_login(id_from_idp=id_from_idp)
super(RASCallback, self).post_login(token_result=token_result)
15 changes: 15 additions & 0 deletions fence/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,9 @@ OPENID_CONNECT:
user_id_field: '' # optional (default "sub"); claims field to get the user_id from
email_field: '' # optional (default "email"); claims field to get the user email from
scope: '' # optional (default "openid")
multifactor_auth_claim_info: # optional, include if you're using arborist to enforce mfa on a per-file level
claim: '' # claims field that indicates mfa, either the acr or acm claim.
values: [ "" ] # possible values that indicate mfa was used. At least one value configured here is required to be in the token
# These Google values must be obtained from Google's Cloud Console
# Follow: https://developers.google.com/identity/protocols/OpenIDConnect
#
Expand Down Expand Up @@ -181,6 +184,9 @@ OPENID_CONNECT:
client_secret: ''
redirect_url: '{{BASE_URL}}/login/ras/callback'
scope: 'openid email profile ga4gh_passport_v1'
# multifactor_auth_claim_info:
# claim: 'acr'
# values: [ 'https://stsstg.nih.gov/assurance/aal/2' ]
# if mock is true, will fake a successful login response for login
# WARNING: DO NOT ENABLE IN PRODUCTION (for testing purposes only)
mock: false
Expand All @@ -207,6 +213,9 @@ OPENID_CONNECT:
# WARNING: DO NOT ENABLE IN PRODUCTION (for testing purposes only)
mock: false
mock_default_user: '[email protected]'
# multifactor_auth_claim_info:
# claim: 'amr'
# values: [ "mfa", "otp", "rsa", "ngcmfa", "wiaormfa" ]
# For information on configuring an Okta tenant as an OIDC IdP refer to Okta documentation at:
# https://developer.okta.com/docs/reference/api/oidc/#2-okta-as-the-identity-platform-for-your-app-or-api
okta:
Expand All @@ -215,6 +224,9 @@ OPENID_CONNECT:
client_secret: ''
redirect_url: '{{BASE_URL}}/login/okta/login/'
scope: 'openid email'
# multifactor_auth_claim_info:
# claim: 'amr'
# values: [ "mfa", "otp", "sms" ]
cognito:
# You must create a user pool in order to have a discovery url
discovery_url: 'https://cognito-idp.{REGION}.amazonaws.com/{USER-POOL-ID}/.well-known/openid-configuration'
Expand All @@ -241,6 +253,9 @@ OPENID_CONNECT:
# WARNING: DO NOT ENABLE IN PRODUCTION (for testing purposes only)
mock: false
mock_default_user: 'http://cilogon.org/serverT/users/64703'
# multifactor_auth_claim_info:
# claim: 'acr'
# values: [ "https://refeds.org/profile/mfa" ]
synapse:
discovery_url: ''
client_id: ''
Expand Down
7 changes: 7 additions & 0 deletions fence/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,13 @@ def post_process(self):
"Visa parsing on login is enabled but `ENABLE_VISA_UPDATE_CRON` is disabled!"
)

for idp_id, idp in self._configs.get("OPENID_CONNECT", {}).items():
mfa_info = idp.get("multifactor_auth_claim_info")
if mfa_info and mfa_info["claim"] not in ["amr", "acr"]:
logger.warning(
f"IdP '{idp_id}' is using multifactor_auth_claim_info '{mfa_info['claim']}', which is neither AMR or ACR. Unable to determine if a user used MFA. Fence will continue and assume they have not used MFA."
)

self._validate_parent_child_studies(self._configs["dbGaP"])

@staticmethod
Expand Down
2 changes: 1 addition & 1 deletion fence/resources/openid/cilogon_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def get_auth_url(self):

return uri

def get_user_id(self, code):
def get_auth_info(self, code):
try:
token_endpoint = self.get_value_from_discovery_doc(
"token_endpoint", "https://cilogon.org/oauth2/token"
Expand Down
2 changes: 1 addition & 1 deletion fence/resources/openid/cognito_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def get_auth_url(self):

return uri

def get_user_id(self, code):
def get_auth_info(self, code):
"""
Exchange code for tokens, get email from id token claims.
Return dict with "email" field on success OR "error" field on error.
Expand Down
2 changes: 1 addition & 1 deletion fence/resources/openid/google_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ def get_auth_url(self):

return uri

def get_user_id(self, code):
def get_auth_info(self, code):
"""
Get user id
"""
Expand Down
44 changes: 42 additions & 2 deletions fence/resources/openid/idp_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ def get_auth_url(self):
)
return uri

def get_user_id(self, code):
def get_auth_info(self, code):
"""
Exchange code for tokens, get user_id from id token claims.
Return dictionary with necessary field(s) for successfully logged in
Expand All @@ -169,7 +169,10 @@ def get_user_id(self, code):
if claims.get(user_id_field):
if user_id_field == "email" and not claims.get("email_verified"):
return {"error": "Email is not verified"}
return {user_id_field: claims[user_id_field]}
return {
user_id_field: claims[user_id_field],
"mfa": self.has_mfa_claim(claims),
}
else:
self.logger.exception(
f"Can't get {user_id_field} from claims: {claims}"
Expand Down Expand Up @@ -220,6 +223,43 @@ def get_access_token(self, user, token_endpoint, db_session=None):

return token_response

def has_mfa_claim(self, decoded_id_token):
"""
Determines if the claim denoting whether multifactor authentication was used is contained within the claims
of the provided id_token.
Parameters:
- decoded_id_token (dict): The decoded id_token, a dict of claims -> claim values.
"""
mfa_claim_info = self.settings.get("multifactor_auth_claim_info")
if not mfa_claim_info:
return False
claim_name = mfa_claim_info.get("claim")
mfa_values = mfa_claim_info.get("values")
if not claim_name or not mfa_values:
self.logger.warning(
f"{self.idp} has a configured multifactor_auth_claim_info with a missing claim name "
f"and values. Please check the OPENID_CONNECT settings for {self.idp} in the fence "
f"config yaml."
)
return False
mfa_claims = []
if claim_name == "amr":
mfa_claims = decoded_id_token.get(claim_name, [])
elif claim_name == "acr":
mfa_claims = decoded_id_token.get(claim_name, "").split(" ")
else:
self.logger.error(
f"{claim_name} is neither AMR or ACR - cannot determine if MFA was used"
)
return False

self.logger.info(
f"Comparing token's {claim_name} claims: {mfa_claims} to mfa values {mfa_values}"
)
return len(set(mfa_claims) & set(mfa_values)) > 0

def store_refresh_token(self, user, refresh_token, expires, db_session=None):
"""
Store refresh token in db.
Expand Down
2 changes: 1 addition & 1 deletion fence/resources/openid/microsoft_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def get_auth_url(self):

return uri

def get_user_id(self, code):
def get_auth_info(self, code):
"""
Get user id given an authorization code
"""
Expand Down
2 changes: 1 addition & 1 deletion fence/resources/openid/okta_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ def get_auth_url(self):

return uri

def get_user_id(self, code):
def get_auth_info(self, code):
try:
token_endpoint = self.get_value_from_discovery_doc(
"token_endpoint",
Expand Down
Loading

0 comments on commit ea885f0

Please sign in to comment.