Skip to content

Commit

Permalink
Merge branch 'integration202006' of https://github.com/uc-cdis/fence
Browse files Browse the repository at this point in the history
…into stable
  • Loading branch information
haraprasadj committed May 29, 2020
2 parents 45b0e36 + c7ddb87 commit b2d0431
Show file tree
Hide file tree
Showing 11 changed files with 401 additions and 90 deletions.
2 changes: 1 addition & 1 deletion Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

@Library('cdis-jenkins-lib@master') _

testPipeline {
testPipeline {
}
58 changes: 35 additions & 23 deletions fence/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from fence.oidc.client import query_client
from fence.oidc.server import server
from fence.resources.aws.boto_manager import BotoManager
from fence.resources.openid.cognito_oauth2 import CognitoOauth2Client as CognitoClient
from fence.resources.openid.google_oauth2 import GoogleOauth2Client as GoogleClient
from fence.resources.openid.microsoft_oauth2 import (
MicrosoftOauth2Client as MicrosoftClient,
Expand Down Expand Up @@ -172,7 +173,7 @@ def public_keys():
def _check_s3_buckets(app):
"""
Function to ensure that all s3_buckets have a valid credential.
Additionally, if there is no region it will produce a warning then trys to fetch and cache the region.
Additionally, if there is no region it will produce a warning then try to fetch and cache the region.
"""
buckets = config.get("S3_BUCKETS") or {}
aws_creds = config.get("AWS_CREDENTIALS") or {}
Expand All @@ -184,7 +185,13 @@ def _check_s3_buckets(app):
raise ValueError(
"No cred for S3_BUCKET: {}. cred is required.".format(bucket_name)
)
if cred not in aws_creds and cred != "*":

# if this is a public bucket, Fence will not try to sign the URL
# so it won't need to know the region.
if cred == "*":
continue

if cred not in aws_creds:
raise ValueError(
"Credential {} for S3_BUCKET {} is not defined in AWS_CREDENTIALS".format(
cred, bucket_name
Expand All @@ -193,30 +200,29 @@ def _check_s3_buckets(app):

# only require region when we're not specifying an
# s3-compatible endpoint URL (ex: no need for region when using cleversafe)
if not bucket_details.get("endpoint_url"):
if not region:
logger.warning(
"WARNING: no region for S3_BUCKET: {}. Providing the region will reduce"
" response time and avoid a call to GetBucketLocation which you make lack the AWS ACLs for.".format(
bucket_name
)
if not region and not bucket_details.get("endpoint_url"):
logger.warning(
"WARNING: no region for S3_BUCKET: {}. Providing the region will reduce"
" response time and avoid a call to GetBucketLocation which you make lack the AWS ACLs for.".format(
bucket_name
)
credential = S3IndexedFileLocation.get_credential_to_access_bucket(
bucket_name,
aws_creds,
config.get("MAX_PRESIGNED_URL_TTL", 3600),
app.boto,
)
credential = S3IndexedFileLocation.get_credential_to_access_bucket(
bucket_name,
aws_creds,
config.get("MAX_PRESIGNED_URL_TTL", 3600),
app.boto,
)
if not getattr(app, "boto"):
logger.warning(
"WARNING: boto not setup for app, probably b/c "
"nothing in AWS_CREDENTIALS. Cannot attempt to get bucket "
"bucket regions."
)
if not getattr(app, "boto"):
logger.warning(
"WARNING: boto not setup for app, probably b/c "
"nothing in AWS_CREDENTIALS. Cannot attempt to get bucket "
"bucket regions."
)
return
return

region = app.boto.get_bucket_region(bucket_name, credential)
config["S3_BUCKETS"][bucket_name]["region"] = region
region = app.boto.get_bucket_region(bucket_name, credential)
config["S3_BUCKETS"][bucket_name]["region"] = region


def app_config(
Expand Down Expand Up @@ -342,6 +348,12 @@ def _setup_oidc_clients(app):
logger=logger,
)

# Add OIDC client for Amazon Cognito if configured.
if "cognito" in oidc:
app.cognito_client = CognitoClient(
oidc["cognito"], HTTP_PROXY=config.get("HTTP_PROXY"), logger=logger
)

# Add OIDC client for multi-tenant fence if configured.
configured_fence = "fence" in oidc and "fence" in enabled_idp_ids
if configured_fence:
Expand Down
10 changes: 9 additions & 1 deletion fence/blueprints/login/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
"""
Create a blueprint with endoints for logins from configured identity providers.
Create a blueprint with endpoints for logins from configured identity providers.
The identity providers include, for example, Google, Shibboleth, or another
fence instance. See the other files in this directory for the definitions of
Expand All @@ -12,6 +12,7 @@

from cdislogging import get_logger

from fence.blueprints.login.cognito import CognitoLogin, CognitoCallback
from fence.blueprints.login.fence_login import FenceLogin, FenceCallback
from fence.blueprints.login.google import GoogleLogin, GoogleCallback
from fence.blueprints.login.shib import ShibbolethLogin, ShibbolethCallback
Expand All @@ -32,6 +33,7 @@
"orcid": "orcid",
"synapse": "synapse",
"microsoft": "microsoft",
"cognito": "cognito",
}


Expand Down Expand Up @@ -266,6 +268,12 @@ def provider_info(login_details):
MicrosoftCallback, "/microsoft/login", strict_slashes=False
)

if "cognito" in configured_idps:
blueprint_api.add_resource(CognitoLogin, "/cognito", strict_slashes=False)
blueprint_api.add_resource(
CognitoCallback, "/cognito/login", strict_slashes=False
)

if "shibboleth" in configured_idps:
blueprint_api.add_resource(ShibbolethLogin, "/shib", strict_slashes=False)
blueprint_api.add_resource(
Expand Down
18 changes: 18 additions & 0 deletions fence/blueprints/login/cognito.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import flask

from fence.blueprints.login.base import DefaultOAuth2Login, DefaultOAuth2Callback
from fence.models import IdentityProvider


class CognitoLogin(DefaultOAuth2Login):
def __init__(self):
super(CognitoLogin, self).__init__(
idp_name=IdentityProvider.cognito, client=flask.current_app.cognito_client
)


class CognitoCallback(DefaultOAuth2Callback):
def __init__(self):
super(CognitoCallback, self).__init__(
idp_name=IdentityProvider.cognito, client=flask.current_app.cognito_client
)
19 changes: 17 additions & 2 deletions fence/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,16 @@ OPENID_CONNECT:
# WARNING: DO NOT ENABLE IN PRODUCTION (for testing purposes only)
mock: false
mock_default_user: 'test@example.com'
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'
client_id: ''
client_secret: ''
redirect_url: '{{BASE_URL}}/login/cognito/login/'
# In the case where Cognito is being used solely as an intermediary to a single IdP,
# and that IdP is a SAML IdP with no 'email_verified' outgoing claim, but it is safe
# to assume all emails from this SAML IdP are in fact verified, we may set this to True
assume_emails_verified: False
shibboleth:
client_id: ''
client_secret: ''
Expand Down Expand Up @@ -231,6 +241,11 @@ LOGIN_OPTIONS: [] # !!! remove the empty list to enable login options!
# idp: orcid
# - name: 'Microsoft Login'
# idp: microsoft
# # Cognito login: You may want to edit the name to reflect Cognito's IdP,
# # especially if Cognito is only using one IdP
# - name: 'Login from Cognito'
# desc: 'Amazon Cognito login'
# idp: cognito
# - name: 'NIH Login'
# idp: fence
# fence_idp: shibboleth
Expand Down Expand Up @@ -487,6 +502,7 @@ AWS_CREDENTIALS: {}

# NOTE: the region is optonal for s3_buckets, however it should be specified to avoid a
# call to GetBucketLocation which you make lack the AWS ACLs for.
# public buckets do not need the region field.
S3_BUCKETS: {}
# NOTE: Remove the {} and supply buckets if needed. Example in comments below
# bucket1:
Expand All @@ -498,8 +514,7 @@ S3_BUCKETS: {}
# cred: 'CRED2'
# region: 'us-east-1'
# bucket3:
# cred: '*'
# region: 'us-east-1'
# cred: '*' # public bucket
# bucket4:
# cred: 'CRED1'
# region: 'us-east-1'
Expand Down
68 changes: 68 additions & 0 deletions fence/resources/openid/cognito_oauth2.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import json
from .idp_oauth2 import Oauth2ClientBase


class CognitoOauth2Client(Oauth2ClientBase):
"""
Amazon Cognito OIDC client
https://docs.aws.amazon.com/cognito/index.html
At time of writing, the Cognito issuer/auth/jwks/token endpoints and
discovery url do not follow the standard OIDC patterns. Furthermore they
depend on the user pool name/ID (and therefore cannot be hardcoded).
So, just pass "" as default values to get_value_from_discovery_doc
and log error when necessary.
"""

def __init__(self, settings, logger, HTTP_PROXY=None):
super(CognitoOauth2Client, self).__init__(
settings,
logger,
scope="openid email",
discovery_url=settings["discovery_url"],
idp="Amazon Cognito",
HTTP_PROXY=HTTP_PROXY,
)

def get_auth_url(self):
"""
Get authorization endpoint from discovery doc
and construct authorization url
"""
authorization_endpoint = self.get_value_from_discovery_doc(
"authorization_endpoint", ""
)
uri, state = self.session.create_authorization_url(
authorization_endpoint, prompt="login"
)

return uri

def get_user_id(self, code):
"""
Exchange code for tokens, get email from id token claims.
Return dict with "email" field on success OR "error" field on error.
"""
try:
token_endpoint = self.get_value_from_discovery_doc("token_endpoint", "")
jwks_endpoint = self.get_value_from_discovery_doc("jwks_uri", "")
claims = self.get_jwt_claims_identity(token_endpoint, jwks_endpoint, code)

self.logger.info(
"Received id token from Cognito:\n{}".format(
json.dumps(claims, indent=4)
)
)

if claims["email"] and (
claims["email_verified"] or self.settings["assume_emails_verified"]
):
return {"email": claims["email"]}
elif claims["email"]:
return {"error": "Email is not verified"}
else:
return {"error": "Can't get email from claims"}

except Exception as e:
self.logger.exception("Can't get user info from Cognito")
return {"error": "Can't get user info from Cognito: {}".format(e)}
14 changes: 11 additions & 3 deletions fence/resources/openid/idp_oauth2.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ def get_value_from_discovery_doc(self, key, default_value):
)
)
return_value = default_value
elif return_value != default_value:
elif return_value != default_value and default_value != "":
self.logger.info(
"{}'s {}, {} differs from our "
"default {}. Using {}'s...".format(
"{}'s discovery doc {}, `{}`, differs from our "
"default, `{}`. Using {}'s...".format(
self.idp, key, return_value, default_value, self.idp
)
)
Expand All @@ -106,6 +106,14 @@ def get_value_from_discovery_doc(self, key, default_value):
)
return_value = default_value

if not return_value:
self.logger.error(
"Could not retrieve `{}` from {} discovery doc {} "
"and default value {} appears to not be set.".format(
key, self.idp, self.discovery_doc.json(), default_value
)
)

return return_value

def get_auth_url(self):
Expand Down
Loading

0 comments on commit b2d0431

Please sign in to comment.