Skip to content

refactor: separate deprecated config for readability #628

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Jul 3, 2023
Merged

refactor: separate deprecated config for readability #628

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3134fc7
refactor: isolate deprecation logic better for readability
consideRatio Jun 29, 2023
b33e00f
docs: add versionremoved directive
consideRatio Jul 3, 2023
6db45b2
docs: update helpstrings to use deprecated and versionremoved directives
consideRatio Jul 3, 2023
6aaa71d
docs: update cilogon's username_claim helpstring to use versionremoved
consideRatio Jul 3, 2023
a29907a
docs: use class relative attribute links for deprecated config
consideRatio Jul 3, 2023
edec67e
docs: update cilogon allowed_idps helpstring
consideRatio Jul 3, 2023
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
21 changes: 9 additions & 12 deletions oauthenticator/auth0.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,6 @@


class Auth0OAuthenticator(OAuthenticator):
_deprecated_oauth_aliases = {
"username_key": ("username_claim", "16.0.0"),
**OAuthenticator._deprecated_oauth_aliases,
}

user_auth_state_key = "auth0_user"

@default("login_service")
Expand Down Expand Up @@ -60,13 +55,6 @@ def _auth0_subdomain_default(self):
# This is allowed to be empty unless auth0_domain is not supplied either
return os.getenv("AUTH0_SUBDOMAIN", "")

username_key = Unicode(
config=True,
help="""
Deprecated, use `Auth0OAuthenticator.username_claim`
""",
)

@default("logout_redirect_url")
def _logout_redirect_url_default(self):
return f"https://{self.auth0_domain}/v2/logout"
Expand All @@ -83,6 +71,15 @@ def _token_url_default(self):
def _userdata_url_default(self):
return f"https://{self.auth0_domain}/userinfo"

# _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
_deprecated_oauth_aliases = {
"username_key": ("username_claim", "16.0.0"),
**OAuthenticator._deprecated_oauth_aliases,
}
username_key = Unicode(
config=True, help="Deprecated, use :attr:`.Auth0OAuthenticator.username_claim`."
)


class LocalAuth0OAuthenticator(LocalAuthenticator, Auth0OAuthenticator):
"""A version that mixes in local system user creation"""
20 changes: 9 additions & 11 deletions oauthenticator/bitbucket.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,6 @@


class BitbucketOAuthenticator(OAuthenticator):
_deprecated_oauth_aliases = {
"team_whitelist": ("allowed_teams", "0.12.0"),
**OAuthenticator._deprecated_oauth_aliases,
}

client_id_env = "BITBUCKET_CLIENT_ID"
client_secret_env = "BITBUCKET_CLIENT_SECRET"
user_auth_state_key = "bitbucket_user"
Expand All @@ -36,18 +31,21 @@ def _token_url_default(self):
def _userdata_url_default(self):
return "https://api.bitbucket.org/2.0/user"

team_whitelist = Set(
allowed_teams = Set(
config=True,
help="""
Deprecated, use `BitbucketOAuthenticator.allowed_teams`
Allow members of selected Bitbucket teams to sign in.
""",
)

allowed_teams = Set(
# _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
_deprecated_oauth_aliases = {
"team_whitelist": ("allowed_teams", "0.12.0"),
**OAuthenticator._deprecated_oauth_aliases,
}
team_whitelist = Set(
config=True,
help="""
Allow members of selected Bitbucket teams to sign in.
""",
help="Deprecated, use :attr:`.BitbucketOAuthenticator.allowed_teams`.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've learned that it's a good practice to always have in VERSION everywhere you have a 'deprecated' or 'removed' message, so that folks who see it know how to coordinate the deprecations with their version requirements without having to dig around and find the relevant version where things changed.

)

async def _fetch_user_teams(self, access_token, token_type):
Expand Down
96 changes: 23 additions & 73 deletions oauthenticator/cilogon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,6 @@ def authorize_redirect(self, *args, **kwargs):


class CILogonOAuthenticator(OAuthenticator):
_deprecated_oauth_aliases = {
# <deprecated-config>:
# (
# <new-config>,
# <deprecation-version>,
# <deprecated-config-and-new-config-have-same-type>
# )
"idp_whitelist": ("allowed_idps", "0.12.0", False),
"idp": ("shown_idps", "15.0.0", False),
"strip_idp_domain": ("allowed_idps", "15.0.0", False),
"shown_idps": ("allowed_idps", "16.0.0", False),
"username_claim": ("allowed_idps", "16.0.0", False),
"additional_username_claims": ("allowed_idps", "16.0.0", False),
**OAuthenticator._deprecated_oauth_aliases,
}

login_handler = CILogonLoginHandler

user_auth_state_key = "cilogon_user"
Expand Down Expand Up @@ -87,14 +71,6 @@ def _token_url(self):
def _userdata_url_default(self):
return f"https://{self.cilogon_host}/oauth2/userinfo"

@default("username_claim")
def _username_claim_default(self):
"""What keys are available will depend on the scopes requested.
See https://www.cilogon.org/oidc for details.
Note that this option can be overridden for specific identity providers via `allowed_idps[<identity provider>]["username_derivation"]["username_claim"]`.
"""
return "eppn"

scope = List(
Unicode(),
default_value=['openid', 'email', 'org.cilogon.userinfo', 'profile'],
Expand Down Expand Up @@ -128,13 +104,6 @@ def _validate_scope(self, proposal):

return scopes

idp_whitelist = List(
config=True,
help="""
Deprecated, use `CIlogonOAuthenticator.allowed_idps`
""",
)

allowed_idps = Dict(
config=True,
help="""
Expand Down Expand Up @@ -231,39 +200,6 @@ def _validate_allowed_idps(self, proposal):

return idps

strip_idp_domain = Bool(
False,
config=True,
help="""
Deprecated, use `CILogonOAuthenticator.allowed_idps[<ipd>]["username_derivation"]["action"] = "strip_idp_domain"`
to enable it and `CIlogonOAuthenticator.allowed_idps[<idp>]["username_derivation"]["domain"]` to list the domain
which will be stripped
""",
)

idp = Unicode(
config=True,
help="""
Deprecated, use `CILogonOAuthenticator.shown_idps`.
""",
)

shown_idps = List(
Unicode(),
config=True,
help="""
Deprecated, `CILogonOAuthenticator.allowed_idps` will determine the idps
shown.

A list of identity providers to be shown as login options. The `idp`
attribute is the SAML Entity ID of the user's selected identity
provider.

See https://cilogon.org/include/idplist.xml for the list of identity
providers supported by CILogon.
""",
)

skin = Unicode(
config=True,
help="""
Expand All @@ -274,16 +210,30 @@ def _validate_allowed_idps(self, proposal):
""",
)

# _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
_deprecated_oauth_aliases = {
"idp_whitelist": ("allowed_idps", "0.12.0", False),
"idp": ("shown_idps", "15.0.0", False),
"strip_idp_domain": ("allowed_idps", "15.0.0", False),
"shown_idps": ("allowed_idps", "16.0.0", False),
"additional_username_claims": ("allowed_idps", "16.0.0", False),
"username_claim": ("allowed_idps", "16.0.0", False),
**OAuthenticator._deprecated_oauth_aliases,
}
idp_whitelist = List(
config=True, help="Removed, use :attr:`.CILogonOAuthenticator.allowed_idps`."
)
idp = Unicode(
config=True, help="Removed, use :attr:`.CILogonOAuthenticator.allowed_idps`."
)
strip_idp_domain = Bool(
config=True, help="Removed, use :attr:`.CILogonOAuthenticator.allowed_idps`."
)
shown_idps = List(
config=True, help="Removed, use :attr:`.CILogonOAuthenticator.allowed_idps`."
)
additional_username_claims = List(
config=True,
help="""
Deprecated, use `CILogonOAuthenticator.allowed_idps["username_derivation"]["username_claim"]`.

Additional claims to check if the username_claim fails.

This is useful for linked identities where not all of them return the
primary username_claim.
""",
config=True, help="Removed, use :attr:`.CILogonOAuthenticator.allowed_idps`."
)

def user_info_to_username(self, user_info):
Expand Down
34 changes: 15 additions & 19 deletions oauthenticator/generic.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,6 @@


class GenericOAuthenticator(OAuthenticator):
_deprecated_oauth_aliases = {
"username_key": ("username_claim", "16.0.0"),
"extra_params": ("token_params", "16.0.0"),
**OAuthenticator._deprecated_oauth_aliases,
}

extra_params = Dict(
config=True,
help="""
Deprecated, use `GenericOAuthenticator.token_params`
""",
)

@default("login_service")
def _login_service_default(self):
return os.environ.get("LOGIN_SERVICE", "OAuth 2.0")
Expand Down Expand Up @@ -69,12 +56,6 @@ def _login_service_default(self):
""",
)

username_key = Union(
[Unicode(os.environ.get('OAUTH2_USERNAME_KEY', 'username')), Callable()],
config=True,
help="""Deprecated, use `GenericOAuthenticator.username_claim`""",
)

username_claim = Union(
[Unicode(os.environ.get('OAUTH2_USERNAME_KEY', 'username')), Callable()],
config=True,
Expand Down Expand Up @@ -105,6 +86,21 @@ def _default_http_client(self):
force_instance=True, defaults=dict(validate_cert=self.tls_verify)
)

# _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
_deprecated_oauth_aliases = {
"username_key": ("username_claim", "16.0.0"),
"extra_params": ("token_params", "16.0.0"),
**OAuthenticator._deprecated_oauth_aliases,
}
username_key = Union(
[Unicode(), Callable()],
config=True,
help="Deprecated, use :attr:`.GenericOAuthenticator.username_claim`.",
)
extra_params = Dict(
config=True, help="Deprecated, use :attr:`.GenericOAuthenticator.token_params`."
)

def user_info_to_username(self, user_info):
"""
Overrides OAuthenticator.user_info_to_username to support the
Expand Down
53 changes: 18 additions & 35 deletions oauthenticator/github.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,6 @@


class GitHubOAuthenticator(OAuthenticator):
_deprecated_oauth_aliases = {
"github_organization_whitelist": ("allowed_organizations", "0.12.0"),
**OAuthenticator._deprecated_oauth_aliases,
}

user_auth_state_key = "github_user"

@default("login_service")
Expand Down Expand Up @@ -99,39 +94,9 @@ def _token_url_default(self):
def _userdata_url_default(self):
return f"{self.github_api}/user"

# deprecated names
github_client_id = Unicode(
config=True,
help="""
DEPRECATED
""",
)

def _github_client_id_changed(self, name, old, new):
self.log.warning("github_client_id is deprecated, use client_id")
self.client_id = new

github_client_secret = Unicode(
config=True,
help="""
DEPRECATED
""",
)

def _github_client_secret_changed(self, name, old, new):
self.log.warning("github_client_secret is deprecated, use client_secret")
self.client_secret = new

client_id_env = 'GITHUB_CLIENT_ID'
client_secret_env = 'GITHUB_CLIENT_SECRET'

github_organization_whitelist = Set(
config=True,
help="""
Deprecated, use `GitHubOAuthenticator.allowed_organizations`
""",
)

allowed_organizations = Set(
config=True,
help="""
Expand Down Expand Up @@ -168,6 +133,24 @@ def _github_client_secret_changed(self, name, old, new):
""",
)

# _deprecated_oauth_aliases is used by deprecation logic in OAuthenticator
_deprecated_oauth_aliases = {
"github_client_id": ("client_id", "0.1.0"),
"github_client_secret": ("client_secret", "0.1.0"),
"github_organization_whitelist": ("allowed_organizations", "0.12.0"),
**OAuthenticator._deprecated_oauth_aliases,
}
github_client_id = Unicode(
config=True, help="Deprecated, use :attr:`.GitHubOAuthenticator.client_id`."
)
github_client_secret = Unicode(
config=True, help="Deprecated, use :attr:`.GitHubOAuthenticator.client_secret`."
)
github_organization_whitelist = Set(
config=True,
help="Deprecated, use :attr:`.GitHubOAuthenticator.allowed_organizations`.",
)

async def check_allowed(self, username, auth_model):
"""
Overrides the OAuthenticator.check_allowed to also allow users part of
Expand Down
Loading