Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/refresh expires in #848

Merged
merged 25 commits into from
Feb 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
b389c7e
store expires_in param from authz request in flask session
mattgarvin1 Nov 11, 2020
d2bf083
take expires_in value from session if present for generating refresh …
mattgarvin1 Nov 11, 2020
161885d
fix key check
mattgarvin1 Nov 11, 2020
f599fd1
debug
mattgarvin1 Nov 11, 2020
972f6d4
revert
mattgarvin1 Nov 13, 2020
0ab6ba1
add exp col to auth code db table model
mattgarvin1 Nov 17, 2020
da913bc
extract expires_in param from authorize request and put in db record
mattgarvin1 Nov 17, 2020
b777895
add refresh_token_expires_in logic to token response
mattgarvin1 Nov 17, 2020
c16aa52
add code for db migration authorization_code table add col
mattgarvin1 Nov 17, 2020
574b2fb
remove comment
mattgarvin1 Nov 17, 2020
b0d655a
reformat
mattgarvin1 Nov 17, 2020
3278fa0
handle pre-db migration case
mattgarvin1 Nov 17, 2020
be165a3
undo last change
mattgarvin1 Nov 17, 2020
1f8701d
handle pre-db-migration case
mattgarvin1 Nov 17, 2020
0c71128
fix init auth code record
mattgarvin1 Nov 17, 2020
eea75dc
undo last change
mattgarvin1 Nov 17, 2020
f62e0e6
revert
mattgarvin1 Nov 17, 2020
776f3aa
try reflecting db to enable backwards compatibility with previous db …
mattgarvin1 Nov 20, 2020
2fa2d01
add ref to relevant doc
mattgarvin1 Nov 20, 2020
eb32ee2
fix db reflection
mattgarvin1 Nov 20, 2020
d7d5d58
add scope handling
mattgarvin1 Nov 20, 2020
4b9a255
reflect auth code table at token endpoint
mattgarvin1 Nov 20, 2020
c792673
revert
mattgarvin1 Dec 2, 2020
b72983b
Accommodate variously named query params for expiration times
vpsx Feb 22, 2021
20f43ff
Refactor get-and-validate-requested-expiration pattern into util fn
vpsx Feb 23, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 28 additions & 17 deletions fence/blueprints/data/blueprint.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
get_signed_url_for_file,
)
from fence.errors import Forbidden, InternalError, UserError, Forbidden
from fence.utils import is_valid_expiration
from fence.utils import get_valid_expiration
from fence.authz.auth import check_arborist_auth


Expand Down Expand Up @@ -165,11 +165,13 @@ def upload_data_file():
blank_index = BlankIndex(
file_name=params["file_name"], authz=params.get("authz"), uploader=uploader
)
expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)

if "expires_in" in params:
is_valid_expiration(params["expires_in"])
expires_in = min(params["expires_in"], expires_in)
expires_in = get_valid_expiration(
params.get("expires_in"),
max_limit=default_expires_in,
default=default_expires_in,
)

response = {
"guid": blank_index.guid,
Expand All @@ -193,10 +195,14 @@ def init_multipart_upload():
if "file_name" not in params:
raise UserError("missing required argument `file_name`")
blank_index = BlankIndex(file_name=params["file_name"])
expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
if "expires_in" in params:
is_valid_expiration(params["expires_in"])
expires_in = min(params["expires_in"], expires_in)

default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
expires_in = get_valid_expiration(
params.get("expires_in"),
max_limit=default_expires_in,
default=default_expires_in,
)

response = {
"guid": blank_index.guid,
"uploadId": BlankIndex.init_multipart_upload(
Expand All @@ -222,10 +228,13 @@ def generate_multipart_upload_presigned_url():
if missing:
raise UserError("missing required arguments: {}".format(list(missing)))

expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
if "expires_in" in params:
is_valid_expiration(params["expires_in"])
expires_in = min(params["expires_in"], expires_in)
default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
expires_in = get_valid_expiration(
params.get("expires_in"),
max_limit=default_expires_in,
default=default_expires_in,
)

response = {
"presigned_url": BlankIndex.generate_aws_presigned_url_for_part(
params["key"],
Expand Down Expand Up @@ -253,10 +262,12 @@ def complete_multipart_upload():
if missing:
raise UserError("missing required arguments: {}".format(list(missing)))

expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
if "expires_in" in params:
is_valid_expiration(params["expires_in"])
expires_in = min(params["expires_in"], expires_in)
default_expires_in = flask.current_app.config.get("MAX_PRESIGNED_URL_TTL", 3600)
expires_in = get_valid_expiration(
params.get("expires_in"),
max_limit=default_expires_in,
default=default_expires_in,
)

try:
BlankIndex.complete_multipart_upload(
Expand Down
9 changes: 5 additions & 4 deletions fence/blueprints/data/indexd.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,10 +64,11 @@ def get_signed_url_for_file(action, file_id, file_name=None):
force_signed_url = False

indexed_file = IndexedFile(file_id)
expires_in = config.get("MAX_PRESIGNED_URL_TTL", 3600)
requested_expires_in = get_valid_expiration_from_request()
if requested_expires_in:
expires_in = min(requested_expires_in, expires_in)
default_expires_in = config.get("MAX_PRESIGNED_URL_TTL", 3600)
expires_in = get_valid_expiration_from_request(
max_limit=default_expires_in,
default=default_expires_in,
)

signed_url = indexed_file.get_signed_url(
requested_protocol,
Expand Down
9 changes: 5 additions & 4 deletions fence/blueprints/storage_creds/google.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,10 +232,11 @@ def handle_user_service_account_creds(self, key, service_account):
"""
# requested time (in seconds) during which the access key will be valid
# x days * 24 hr/day * 60 min/hr * 60 s/min = y seconds
expires_in = cirrus_config.SERVICE_KEY_EXPIRATION_IN_DAYS * 24 * 60 * 60
requested_expires_in = get_valid_expiration_from_request()
if requested_expires_in:
expires_in = min(expires_in, requested_expires_in)
default_expires_in = cirrus_config.SERVICE_KEY_EXPIRATION_IN_DAYS * 24 * 60 * 60
expires_in = get_valid_expiration_from_request(
max_limit=default_expires_in,
default=default_expires_in,
)

expiration_time = int(time.time()) + int(expires_in)
key_id = key.get("private_key_id")
Expand Down
9 changes: 9 additions & 0 deletions fence/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,8 @@ class AuthorizationCode(Base, OAuth2AuthorizationCodeMixin):

nonce = Column(String, nullable=True)

refresh_token_expires_in = Column(Integer, nullable=True)

_scope = Column(Text, default="")

def __init__(self, **kwargs):
Expand Down Expand Up @@ -660,6 +662,13 @@ def migrate(driver):
metadata=md,
)

add_column_if_not_exist(
table_name=AuthorizationCode.__tablename__,
column=Column("refresh_token_expires_in", Integer),
driver=driver,
metadata=md,
)

drop_foreign_key_column_if_exist(
table_name=GoogleProxyGroup.__tablename__,
column_name="user_id",
Expand Down
14 changes: 13 additions & 1 deletion fence/oidc/grants/oidc_code_grant.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
)
from authlib.oauth2.rfc6749 import InvalidRequestError
import flask

from fence.utils import get_valid_expiration_from_request
from fence.config import config
from fence.models import AuthorizationCode, ClientAuthType, User


Expand Down Expand Up @@ -35,13 +36,22 @@ def create_authorization_code(client, grant_user, request):
the arguments passed from the OAuth request (the redirect URI, scope,
and nonce).
"""

# requested lifetime (in seconds) for the refresh token
refresh_token_expires_in = get_valid_expiration_from_request(
expiry_param="refresh_token_expires_in",
max_limit=config["REFRESH_TOKEN_EXPIRES_IN"],
default=config["REFRESH_TOKEN_EXPIRES_IN"],
)

code = AuthorizationCode(
code=generate_token(50),
client_id=client.client_id,
redirect_uri=request.redirect_uri,
scope=request.scope,
user_id=grant_user.id,
nonce=request.data.get("nonce"),
refresh_token_expires_in=refresh_token_expires_in,
)

with flask.current_app.db.session as session:
Expand All @@ -63,6 +73,7 @@ def create_token_response(self):

scope = authorization_code.scope
nonce = authorization_code.nonce
refresh_token_expires_in = authorization_code.refresh_token_expires_in

token = self.generate_token(
client,
Expand All @@ -71,6 +82,7 @@ def create_token_response(self):
scope=scope,
include_refresh_token=client.has_client_secret(),
nonce=nonce,
refresh_token_expires_in=refresh_token_expires_in,
)

self.request.user = user
Expand Down
5 changes: 4 additions & 1 deletion fence/oidc/jwt_generator.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ def generate_token_response(
client,
grant_type,
expires_in=None,
refresh_token_expires_in=None,
user=None,
scope=None,
include_refresh_token=True,
Expand Down Expand Up @@ -201,11 +202,13 @@ def generate_token_response(
# If ``refresh_token`` was passed (for instance from the refresh
# grant), use that instead of generating a new one.
if refresh_token is None:
if refresh_token_expires_in is None:
refresh_token_expires_in = config["REFRESH_TOKEN_EXPIRES_IN"]
refresh_token = generate_signed_refresh_token(
kid=keypair.kid,
private_key=keypair.private_key,
user=user,
expires_in=config["REFRESH_TOKEN_EXPIRES_IN"],
expires_in=refresh_token_expires_in,
scopes=scope,
client_id=client.client_id,
).token
Expand Down
19 changes: 10 additions & 9 deletions fence/resources/google/access_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,18 +851,19 @@ def add_user_service_account_to_db(session, to_add_project_ids, service_account)

access_groups = _get_google_access_groups(session, project_id)

# timestamp at which the SA will lose bucket access
# time until the SA will lose bucket access
# by default: use configured time or 7 days
expiration_time = int(time.time()) + config.get(
default_expires_in = config.get(
"GOOGLE_USER_SERVICE_ACCOUNT_ACCESS_EXPIRES_IN", 604800
)
requested_expires_in = (
get_valid_expiration_from_request()
) # requested time (in seconds)
if requested_expires_in:
# convert it to timestamp
requested_expiration = int(time.time()) + requested_expires_in
expiration_time = min(expiration_time, requested_expiration)
# use expires_in from request query params if it was provided and
# it was not greater than the default
expires_in = get_valid_expiration_from_request(
max_limit=default_expires_in,
default=default_expires_in,
)
# convert expires_in to timestamp
expiration_time = int(time.time() + expires_in)

for access_group in access_groups:
sa_to_group = ServiceAccountToGoogleBucketAccessGroup(
Expand Down
17 changes: 9 additions & 8 deletions fence/scripting/fence_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@
from fence.scripting.google_monitor import email_users_without_access, validation_check
from fence.config import config
from fence.sync.sync_users import UserSyncer
from fence.utils import create_client, is_valid_expiration
from fence.utils import create_client, get_valid_expiration

logger = get_logger(__name__)

Expand Down Expand Up @@ -1444,16 +1444,17 @@ def force_update_google_link(DB, username, google_email, expires_in=None):
user_id, google_email, session
)

# timestamp at which the SA will lose bucket access
# time until the SA will lose bucket access
# by default: use configured time or 7 days
expiration = int(time.time()) + config.get(
default_expires_in = config.get(
"GOOGLE_USER_SERVICE_ACCOUNT_ACCESS_EXPIRES_IN", 604800
)
if expires_in:
is_valid_expiration(expires_in)
# convert it to timestamp
requested_expiration = int(time.time()) + expires_in
expiration = min(expiration, requested_expiration)
# use expires_in from arg if it was provided and it was not greater than the default
expires_in = get_valid_expiration(
expires_in, max_limit=default_expires_in, default=default_expires_in
)
# convert expires_in to timestamp
expiration = int(time.time() + expires_in)

force_update_user_google_account_expiration(
user_google_account, proxy_group_id, google_email, expiration, session
Expand Down
39 changes: 26 additions & 13 deletions fence/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -275,27 +275,40 @@ def send_email(from_email, to_emails, subject, text, smtp_domain):
)


def get_valid_expiration_from_request():
def get_valid_expiration_from_request(
expiry_param="expires_in", max_limit=None, default=None
):
"""
Return the expires_in param if it is in the request, None otherwise.
Throw an error if the requested expires_in is not a positive integer.
Thin wrapper around get_valid_expiration; looks for default query parameter "expires_in"
in flask request, unless a different parameter name was specified.
"""
if "expires_in" in flask.request.args:
is_valid_expiration(flask.request.args["expires_in"])
return int(flask.request.args["expires_in"])
else:
return None
return get_valid_expiration(
flask.request.args.get(expiry_param), max_limit=max_limit, default=default
)


def is_valid_expiration(expires_in):
def get_valid_expiration(requested_expiration, max_limit=None, default=None):
"""
Throw an error if expires_in is not a positive integer.
If requested_expiration is not a positive integer and not None, throw error.
If max_limit is provided and requested_expiration exceeds max_limit,
return max_limit.
If requested_expiration is None, return default (which may also be None).
Else return requested_expiration.
"""
if requested_expiration is None:
return default
try:
expires_in = int(flask.request.args["expires_in"])
assert expires_in > 0
rv = int(requested_expiration)
assert rv > 0
if max_limit:
rv = min(rv, max_limit)
return rv
except (ValueError, AssertionError):
raise UserError("expires_in must be a positive integer")
raise UserError(
"Requested expiry must be a positive integer; instead got {}".format(
requested_expiration
)
)


def _print_func_name(function):
Expand Down