Skip to content

Commit

Permalink
PXP-10250 PXP-10268 OIDC clients expliration (#1057)
Browse files Browse the repository at this point in the history
  • Loading branch information
paulineribeyre committed Nov 30, 2022
1 parent 38e6e21 commit 254dac3
Show file tree
Hide file tree
Showing 7 changed files with 367 additions and 13 deletions.
6 changes: 3 additions & 3 deletions .secrets.baseline
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@
"filename": "fence/utils.py",
"hashed_secret": "8318df9ecda039deac9868adf1944a29a95c7114",
"is_verified": false,
"line_number": 110
"line_number": 112
}
],
"migrations/versions/e4c7b0ab68d3_create_tables.py": [
Expand Down Expand Up @@ -321,7 +321,7 @@
"filename": "tests/scripting/test_fence-create.py",
"hashed_secret": "e5e9fa1ba31ecd1ae84f75caaa474f3a663f05f4",
"is_verified": false,
"line_number": 221
"line_number": 264
}
],
"tests/test-fence-config.yaml": [
Expand All @@ -334,5 +334,5 @@
}
]
},
"generated_at": "2022-08-12T15:25:33Z"
"generated_at": "2022-11-10T22:37:01Z"
}
25 changes: 20 additions & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -491,9 +491,11 @@ WARNING: fence-create directly modifies the database in some cases and may circu
As a Gen3 commons administrator, if you want to create an oauth client that skips user consent step, use the following command:

```bash
fence-create client-create --client CLIENT_NAME --urls OAUTH_REDIRECT_URL --username USERNAME --auto-approve
fence-create client-create --client CLIENT_NAME --urls OAUTH_REDIRECT_URL --username USERNAME --auto-approve (--expires-in 30)
```

The optional `--expires-in` parameter allows specifying the number of days until this client expires.

#### Register an Implicit Oauth Client

As a Gen3 commons administrator, if you want to create an implicit oauth client for a webapp:
Expand Down Expand Up @@ -521,7 +523,7 @@ The OAuth2 Client Credentials flow is used for machine-to-machine communication
As a Gen3 commons administrator, if you want to create an OAuth client for a client credentials flow:

```bash
fence-create client-create --client CLIENT_NAME --grant-types client_credentials
fence-create client-create --client CLIENT_NAME --grant-types client_credentials (--expires-in 30)
```

This command will return a client ID and client secret, which you can then use to obtain an access token:
Expand All @@ -530,12 +532,12 @@ This command will return a client ID and client secret, which you can then use t
curl --request POST https://FENCE_URL/oauth2/token?grant_type=client_credentials -d scope="openid user" --user CLIENT_ID:CLIENT_SECRET
```

The optional `--expires-in` parameter allows specifying the number of *days* until this client expires. The recommendation is to rotate credentials with the `client_credentials` grant at least once a year.

NOTE: In Gen3, you can grant specific access to a client the same way you would to a user. See the [user.yaml guide](https://github.com/uc-cdis/fence/blob/master/docs/user.yaml_guide.md) for more details.

NOTE: Client credentials tokens are not linked to a user. They are not supported by all Gen3 endpoints.

NOTE: The recommendation is to rotate these credentials at least once a year. Credentials expiration is not enforced at the moment but may be in the future.

#### Modify OAuth Client

```bash
Expand All @@ -547,7 +549,7 @@ allowed here too.

Add `--append` argument to add new callback urls or allowed scopes to existing client (instead of replacing them) using `--append --urls` or `--append --allowed-scopes`
```bash
fence-create client-modify --client CLIENT_NAME --urls http://localhost/api/v0/new/oauth2/authorize --append
fence-create client-modify --client CLIENT_NAME --urls http://localhost/api/v0/new/oauth2/authorize --append (--expires-in 30)
```

#### Delete OAuth Client
Expand All @@ -557,6 +559,19 @@ fence-create client-delete --client CLIENT_NAME
```
That command should output the result of the deletion attempt.

#### Delete Expired OAuth Clients

```bash
fence-create client-delete-expired
```

To post a warning in Slack about any clients that expired or are about to expire:

```bash
fence-create client-delete-expired --slack-webhook <url> --warning-days <default 7: only post about clients expiring in under 7 days>
```


#### List OAuth Clients

```bash
Expand Down
24 changes: 24 additions & 0 deletions bin/fence_create.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
create_google_logging_bucket,
create_sample_data,
delete_client_action,
delete_expired_clients_action,
delete_users,
delete_expired_google_access,
cleanup_expired_ga4gh_information,
Expand Down Expand Up @@ -103,6 +104,9 @@ def parse_arguments():
client_create.add_argument(
"--allowed-scopes", help="which scopes are allowed for this client", nargs="+"
)
client_create.add_argument(
"--expires-in", help="days until this client expires", required=False
)

client_modify = subparsers.add_parser("client-modify")
client_modify.add_argument("--client", required=True)
Expand Down Expand Up @@ -137,12 +141,28 @@ def parse_arguments():
"previous policies will be revoked",
nargs="*",
)
client_modify.add_argument(
"--expires-in", help="days until this client expires", required=False
)

client_list = subparsers.add_parser("client-list")

client_delete = subparsers.add_parser("client-delete")
client_delete.add_argument("--client", required=True)

client_delete_expired = subparsers.add_parser("client-delete-expired")
client_delete_expired.add_argument(
"--slack-webhook",
help="Slack webhook to post warnings when clients expired or are about to expire",
required=False,
)
client_delete_expired.add_argument(
"--warning-days",
help="how many days before a client expires should we post a warning on Slack",
required=False,
default=7,
)

user_delete = subparsers.add_parser("user-delete")
user_delete.add_argument("--users", required=True, nargs="+")

Expand Down Expand Up @@ -433,6 +453,7 @@ def main():
arborist=arborist,
policies=args.policies,
allowed_scopes=args.allowed_scopes,
expires_in=args.expires_in,
)
elif args.action == "client-modify":
modify_client_action(
Expand All @@ -448,11 +469,14 @@ def main():
policies=args.policies,
allowed_scopes=args.allowed_scopes,
append=args.append,
expires_in=args.expires_in,
)
elif args.action == "client-delete":
delete_client_action(DB, args.client)
elif args.action == "client-list":
list_client_action(DB)
elif args.action == "client-delete-expired":
delete_expired_clients_action(DB, args.slack_webhook, args.warning_days)
elif args.action == "user-delete":
delete_users(DB, args.users)
elif args.action == "expired-service-account-delete":
Expand Down
44 changes: 43 additions & 1 deletion fence/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

from authlib.flask.oauth2.sqla import OAuth2AuthorizationCodeMixin, OAuth2ClientMixin
import bcrypt
from datetime import datetime, timedelta
import flask
from sqlalchemy import (
Integer,
BigInteger,
Expand Down Expand Up @@ -51,6 +53,7 @@

from fence import logger
from fence.config import config
from fence.errors import UserError


def query_for_user(session, username):
Expand Down Expand Up @@ -123,6 +126,37 @@ def get_project_to_authz_mapping(session):
return output


def get_client_expires_at(expires_in, grant_types):
"""
Given an `expires_in` value (days from now), return an `expires_at` value (timestamp).
expires_in (int/float/str): days until this client expires
grant_types (str): list of the client's grants joined by "\n"
"""
expires_at = None

if expires_in:
try:
expires_in = float(expires_in)
assert expires_in > 0
except (ValueError, AssertionError):
raise UserError(
f"Requested expiry must be a positive integer; instead got: {expires_in}"
)

# for backwards compatibility, 0 means no expiration
if expires_in != 0:
expires_at = (datetime.utcnow() + timedelta(days=expires_in)).timestamp()

if "client_credentials" in grant_types.split("\n"):
if not expires_in or expires_in <= 0 or expires_in > 366:
logger.warning(
"Credentials with the 'client_credentials' grant which will be used externally are required to expire within 12 months. Use the `--expires-in` parameter to add an expiration."
)

return expires_at


class ClientAuthType(Enum):
"""
List the possible types of OAuth client authentication, which are
Expand Down Expand Up @@ -184,9 +218,11 @@ class Client(Base, OAuth2ClientMixin):
_default_scopes = Column(Text)
_scopes = ["compute", "storage", "user"]

expires_at = Column(Integer, nullable=False, default=0)

# note that authlib adds a response_type column which is not used here

def __init__(self, client_id, **kwargs):
def __init__(self, client_id, expires_in=0, **kwargs):
"""
NOTE that for authlib, the client must have an attribute ``redirect_uri`` which
is a newline-delimited list of valid redirect URIs.
Expand Down Expand Up @@ -233,6 +269,12 @@ def __init__(self, client_id, **kwargs):
"redirect_uri"
), "Redirect URL(s) are required for the 'authorization_code' grant"

expires_at = get_client_expires_at(
expires_in=expires_in, grant_types=kwargs["grant_type"]
)
if expires_at:
kwargs["expires_at"] = expires_at

super(Client, self).__init__(client_id=client_id, **kwargs)

@property
Expand Down

0 comments on commit 254dac3

Please sign in to comment.