Skip to content

Commit

Permalink
revised group expiration
Browse files Browse the repository at this point in the history
  • Loading branch information
flashguerdon committed Jan 24, 2025
1 parent 7c5cbe4 commit 8c4240a
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 129 deletions.
61 changes: 4 additions & 57 deletions fence/blueprints/login/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,9 @@ def get(self):

resp = _login(username, self.idp_name, email=email, id_from_idp=id_from_idp)

if not flask.g.user:
raise UserError("Authentication failed: flask.g.user is missing.")

This comment has been minimized.

Copy link
@Avantol13

Avantol13 Jan 24, 2025

Contributor

In what situation did this occur? If we need this, then we should add a regression unit test (since this code has been in use for a while). And we need the error to be less internally specific. UserError is a 400, so the error needs to be informative to the end user logging in b/c it will show up on their screen. We don't want to expose internal details like flask.g


expires = self.extract_exp(refresh_token)

# if the access token is not a JWT, or does not carry exp,
Expand Down Expand Up @@ -231,17 +234,7 @@ def extract_exp(self, refresh_token):
except Exception as e:
logger.info(f"Refresh token expiry: Method (PyJWT) failed: {e}")

# Method 2: Introspection
try:
introspection_response = self.introspect_token(refresh_token)
exp = introspection_response.get("exp")

if exp is not None:
return exp
except Exception as e:
logger.info(f"Refresh token expiry: Method Introspection failed: {e}")

# Method 3: Manual base64 decoding
# Method 2: Manual base64 decoding
try:
# Assuming the token is a JWT (header.payload.signature)
payload_encoded = refresh_token.split(".")[1]
Expand All @@ -259,50 +252,6 @@ def extract_exp(self, refresh_token):
# If all methods fail, return None
return None

def introspect_token(self, token):
"""Introspects an access token to determine its validity and retrieve associated metadata.
This method sends a POST request to the introspection endpoint specified in the OpenID
discovery document. The request includes the provided token and client credentials,
allowing verification of the token's validity and retrieval of any additional metadata
(e.g., token expiry, scopes, or user information).
Args:
token (str): The access token to be introspected.
Returns:
dict or None: A dictionary containing the token's introspection data if the request
is successful and the response status code is 200. If the introspection fails or an
exception occurs, returns None.
Raises:
Exception: Logs an error message if an error occurs during the introspection process.
"""
try:
introspect_endpoint = self.client.get_value_from_discovery_doc(
"introspection_endpoint", ""
)

# Headers and payload for the introspection request
headers = {"Content-Type": "application/x-www-form-urlencoded"}
data = {
"token": token,
"client_id": flask.session.get("client_id"),
"client_secret": flask.session.get("client_secret"),
}

response = requests.post(introspect_endpoint, headers=headers, data=data)

if response.status_code == 200:
return response.json()
else:
logger.info(f"Error introspecting token: {response.status_code}")
return None

except Exception as e:
logger.info(f"Error introspecting token: {e}")
return None

def post_login(self, user=None, token_result=None, **kwargs):
prepare_login_log(self.idp_name)

Expand All @@ -314,8 +263,6 @@ def post_login(self, user=None, token_result=None, **kwargs):
client_id=flask.session.get("client_id"),
)

# this attribute is only applicable to some OAuth clients
# (e.g., not all clients need is_read_authz_groups_from_tokens_enabled)
if self.read_authz_groups_from_tokens:
self.client.update_user_authorization(
user=user, pkey_cache=None, db_session=None, idp_name=self.idp_name
Expand Down
2 changes: 2 additions & 0 deletions fence/config-default.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,8 @@ OPENID_CONNECT:
verify_aud: true
# This specifies the expected audience (aud) value for the JWT, ensuring that the token is intended for use with the 'fence' service.
audience: fence
# default refresh token expiration duration
default_refresh_token_exp: 3600
# These Google values must be obtained from Google's Cloud Console
# Follow: https://developers.google.com/identity/protocols/OpenIDConnect
#
Expand Down
7 changes: 1 addition & 6 deletions fence/error_handler.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,7 @@ def get_error_response(error: Exception):
)
)

# Decide whether to re-raise errors or handle gracefully based on the debug flag
debug_mode = config.get("DEBUG", False)

if debug_mode:
# Re-raise the error in debug mode for troubleshooting
raise error
# raise error

This comment has been minimized.

Copy link
@Avantol13

Avantol13 Jan 24, 2025

Contributor

I think we can remove this too


# Prepare user-facing message
message = details.get("message")
Expand Down
129 changes: 63 additions & 66 deletions fence/resources/openid/idp_oauth2.py
Original file line number Diff line number Diff line change
@@ -1,19 +1,20 @@
from email.policy import default

from authlib.integrations.requests_client import OAuth2Session
from boto3 import client
from cached_property import cached_property
from flask import current_app
from jose import jwt

from jose.exceptions import JWTError, JWTClaimsError
import requests
import time
import datetime
import backoff
import jwt
from fence.utils import DEFAULT_BACKOFF_SETTINGS
from fence.errors import AuthError
from fence.models import UpstreamRefreshToken

from fence.jwt.validate import validate_jwt
from authutils.token.keys import get_public_key_for_token


class Oauth2ClientBase(object):
"""
Expand Down Expand Up @@ -96,71 +97,45 @@ def get_jwt_keys(self, jwks_uri):
return None
return resp.json()["keys"]

def decode_and_validate_token(self, token_id, keys, audience, verify_aud=True):
"""Decodes and validates a JWT (JSON Web Token) using provided keys and audience.
This function decodes a JWT and validates its signature and audience claim,
if required. It is typically used for tokens that require validation to
ensure integrity and authenticity.
Args:
token_id (str): The JWT token to decode.
keys (list): A list of keys to use for decoding the token, usually
provided by the Identity Provider (IdP).
audience (str): The expected audience (`aud`) claim to verify within the token.
verify_aud (bool, optional): Flag to enable or disable audience verification.
Defaults to True.
Returns:
dict: A dictionary of validated token claims if decoding and validation are successful.
Raises:
JWTClaimsError: If the token's claims, such as audience, do not match the expected values.
JWTError: If there is an error with the JWT structure or verification.
Notes:
- This function assumes the token is signed using the RS256 algorithm.
- Audience verification (`aud`) is performed if `verify_aud` is set to True.
"""
try:
validated_claims = jwt.decode(
token_id,
keys,
options={"verify_aud": verify_aud, "verify_at_hash": False},
algorithms=["RS256"],
audience=audience,
)
self.logger.info("Token decoded and validated successfully.")
return validated_claims
except JWTClaimsError as e:
self.logger.error(f"Claim error: {e}")
raise JWTClaimsError(f"Invalid audience: {e}")
except JWTError as e:
self.logger.error(f"JWT error: {e}")
raise JWTError(f"JWT error occurred: {e}")

def get_jwt_claims_identity(self, token_endpoint, jwks_endpoint, code):
"""
Get jwt identity claims
"""

token = self.get_token(token_endpoint, code)

keys = self.get_jwt_keys(jwks_endpoint)

refresh_token = token.get("refresh_token", None)

# Extract issuer from the token without signature verification
try:
decoded_token = jwt.decode(
token["id_token"],
options={"verify_signature": False},
algorithms=["RS256"],
key="",
)
issuer = decoded_token.get("iss")
except JWTError as e:
raise JWTError(f"Invalid token: {e}")

# validate audience and hash. also ensure that the algorithm is correctly derived from the token.
# hash verification has not been implemented yet
verify_aud = self.settings.get("verify_aud", False)
audience = self.settings.get("audience", self.settings.get("client_id"))
return (
self.decode_and_validate_token(
token["id_token"], keys, audience, verify_aud
),
refresh_token,

decoded_token = validate_jwt(
encoded_token=token["id_token"],
aud=audience,
scope=None,
issuers=[issuer],
purpose=None,
require_purpose=False,
options={"verify_aud": verify_aud, "verify_hash": False},
attempt_refresh=True,
)

return decoded_token, refresh_token

def get_value_from_discovery_doc(self, key, default_value):
"""
Given a key return a value by the recommended method of
Expand Down Expand Up @@ -316,8 +291,11 @@ def get_access_token(self, user, token_endpoint, db_session=None):

refresh_token = token_response["refresh_token"]
# Fetching the expires at from token_response.
# Defaulting to 1 hour if not available.
expires_at = token_response.get("expires_at", time.time() + 3600)
# Defaulting to config settings.
default_refresh_token_exp = self.settings.get("default_refresh_token_exp")
expires_at = token_response.get(
"expires_at", time.time() + default_refresh_token_exp
)

self.store_refresh_token(
user,
Expand Down Expand Up @@ -382,6 +360,9 @@ def store_refresh_token(self, user, refresh_token, expires, db_session=None):
current_db_session = db_session.object_session(upstream_refresh_token)
current_db_session.add(upstream_refresh_token)
db_session.commit()
self.logger.info(
f"Refresh token has been persisted for user: {user} , with expiration of {expires}"
)

def get_groups_from_token(self, decoded_id_token, group_prefix=""):
"""
Expand Down Expand Up @@ -457,22 +438,28 @@ def update_user_authorization(self, user, pkey_cache, db_session=None, **kwargs)

# this get_access_token also persists the refresh token in the db
token = self.get_access_token(user, token_endpoint, db_session)
jwks_endpoint = self.get_value_from_discovery_doc("jwks_uri", "")
keys = self.get_jwt_keys(jwks_endpoint)
expires_at = token["expires_at"]

verify_aud = self.settings.get("verify_aud", False)
audience = self.settings.get("audience", self.settings.get("client_id"))
decoded_token_id = self.decode_and_validate_token(
token_id=token["id_token"],
keys=keys,

key = get_public_key_for_token(
token["id_token"], attempt_refresh=True, pkey_cache={}
)

decoded_token_id = jwt.decode(
token["id_token"],
key=key,
options={"verify_aud": verify_aud, "verify_at_hash": False},
algorithms=["RS256"],
audience=audience,
verify_aud=verify_aud,
)
self.logger.info("Token decoded and validated successfully.")

except Exception as e:
err_msg = "Could not refresh token"
self.logger.exception("{}: {}".format(err_msg, e))
raise

if self.read_authz_groups_from_tokens:
group_prefix = self.settings.get("authz_groups_sync", {}).get(
"group_prefix", ""
Expand All @@ -486,8 +473,6 @@ def update_user_authorization(self, user, pkey_cache, db_session=None, **kwargs)
decoded_token_id, group_prefix
)

exp = datetime.datetime.fromtimestamp(expires_at, tz=datetime.timezone.utc)

# if group name is in the list from arborist:
if authz_groups_from_idp:
authz_groups_from_idp = [
Expand All @@ -501,10 +486,22 @@ def update_user_authorization(self, user, pkey_cache, db_session=None, **kwargs)
group_membership_duration = self.settings.get(
"group_membership_expiration_duration", 3600 * 24 * 7
)
group_membership_expires_at = datetime.datetime.now(

# Get the refresh token expiration from the token response
refresh_token_expires_at = datetime.datetime.fromtimestamp(
token.get("expires_at", time.time()), tz=datetime.timezone.utc
)

# Calculate the configured group membership expiration
configured_expires_at = datetime.datetime.now(
tz=datetime.timezone.utc
) + datetime.timedelta(seconds=group_membership_duration)

# Ensure group membership does not exceed refresh token expiration
group_membership_expires_at = min(
refresh_token_expires_at, configured_expires_at
)

# Add user to all matching groups from IDP
for arborist_group in arborist_groups:
if arborist_group["name"] in idp_group_names:
Expand Down
2 changes: 2 additions & 0 deletions tests/test-fence-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,8 @@ OPENID_CONNECT:
verify_aud: false
# This specifies the expected audience (aud) value for the JWT, ensuring that the token is intended for use with the 'fence' service.
audience: fence
# default refresh token expiration duration
default_refresh_token_exp: 3600

# these are the *possible* scopes a client can be given, NOT scopes that are
# given to all clients. You can be more restrictive during client creation
Expand Down

0 comments on commit 8c4240a

Please sign in to comment.