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.

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

move calls to get_settings outside of __init__ #496

Closed
wants to merge 1 commit into from
Closed

move calls to get_settings outside of __init__ #496

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
51 changes: 29 additions & 22 deletions mozilla_django_oidc/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.core.exceptions import ImproperlyConfigured, SuspiciousOperation
from django.urls import reverse
from django.utils.encoding import force_bytes, smart_bytes, smart_str
from django.utils.functional import cached_property
from django.utils.module_loading import import_string
from josepy.b64 import b64decode
from josepy.jwk import JWK
Expand Down Expand Up @@ -44,21 +45,27 @@ class OIDCAuthenticationBackend(ModelBackend):

def __init__(self, *args, **kwargs):
"""Initialize settings."""
self.OIDC_OP_TOKEN_ENDPOINT = self.get_settings("OIDC_OP_TOKEN_ENDPOINT")
self.OIDC_OP_USER_ENDPOINT = self.get_settings("OIDC_OP_USER_ENDPOINT")
self.OIDC_OP_JWKS_ENDPOINT = self.get_settings("OIDC_OP_JWKS_ENDPOINT", None)
self.OIDC_RP_CLIENT_ID = self.get_settings("OIDC_RP_CLIENT_ID")
self.OIDC_RP_CLIENT_SECRET = self.get_settings("OIDC_RP_CLIENT_SECRET")
self.OIDC_RP_SIGN_ALGO = self.get_settings("OIDC_RP_SIGN_ALGO", "HS256")
self.OIDC_RP_IDP_SIGN_KEY = self.get_settings("OIDC_RP_IDP_SIGN_KEY", None)

if self.OIDC_RP_SIGN_ALGO.startswith("RS") and (
self.OIDC_RP_IDP_SIGN_KEY is None and self.OIDC_OP_JWKS_ENDPOINT is None
self.UserModel = get_user_model()

@cached_property
def settings(self):
retval = {
Comment on lines +50 to +52
Copy link

Choose a reason for hiding this comment

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

@stevenbal is this going to break our library?

Copy link

@stevenbal stevenbal Feb 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

With the way our library is implemented now: yes, because we rely on overriding get_settings

Copy link

Choose a reason for hiding this comment

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

I don't think this is too much of a problem? I don't see any changes to that method in particular and instead it will allow us to remove our __init__ override since settings are now consumed lazily. So, what am I missing? 😄

Copy link

Choose a reason for hiding this comment

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

Whoops, I skimmed over it too quickly and thought get_settings was removed here altogether 😅. You're right, this should work and the init override can be removed.

"OIDC_OP_TOKEN_ENDPOINT": self.get_settings("OIDC_OP_TOKEN_ENDPOINT"),
"OIDC_OP_USER_ENDPOINT": self.get_settings("OIDC_OP_USER_ENDPOINT"),
"OIDC_OP_JWKS_ENDPOINT": self.get_settings("OIDC_OP_JWKS_ENDPOINT", None),
"OIDC_RP_CLIENT_ID": self.get_settings("OIDC_RP_CLIENT_ID"),
"OIDC_RP_CLIENT_SECRET": self.get_settings("OIDC_RP_CLIENT_SECRET"),
"OIDC_RP_SIGN_ALGO": self.get_settings("OIDC_RP_SIGN_ALGO", "HS256"),
"OIDC_RP_IDP_SIGN_KEY": self.get_settings("OIDC_RP_IDP_SIGN_KEY", None),
}

if retval["OIDC_RP_SIGN_ALGO"].startswith("RS") and (
retval["OIDC_RP_IDP_SIGN_KEY"] is None and retval["OIDC_OP_JWKS_ENDPOINT"] is None
):
msg = "{} alg requires OIDC_RP_IDP_SIGN_KEY or OIDC_OP_JWKS_ENDPOINT to be configured."
raise ImproperlyConfigured(msg.format(self.OIDC_RP_SIGN_ALGO))
raise ImproperlyConfigured(msg.format(retval["OIDC_RP_SIGN_ALGO"]))

self.UserModel = get_user_model()
return retval

@staticmethod
def get_settings(attr, *args):
Expand Down Expand Up @@ -123,7 +130,7 @@ def _verify_jws(self, payload, key):
msg = "No alg value found in header"
raise SuspiciousOperation(msg)

if alg != self.OIDC_RP_SIGN_ALGO:
if alg != self.settings["OIDC_RP_SIGN_ALGO"]:
msg = (
"The provider algorithm {!r} does not match the client's "
"OIDC_RP_SIGN_ALGO.".format(alg)
Expand All @@ -146,7 +153,7 @@ def _verify_jws(self, payload, key):
def retrieve_matching_jwk(self, token):
"""Get the signing key by exploring the JWKS endpoint of the OP."""
response_jwks = requests.get(
self.OIDC_OP_JWKS_ENDPOINT,
self.settings["OIDC_OP_JWKS_ENDPOINT"],
verify=self.get_settings("OIDC_VERIFY_SSL", True),
timeout=self.get_settings("OIDC_TIMEOUT", None),
proxies=self.get_settings("OIDC_PROXY", None),
Expand Down Expand Up @@ -190,13 +197,13 @@ def verify_token(self, token, **kwargs):
nonce = kwargs.get("nonce")

token = force_bytes(token)
if self.OIDC_RP_SIGN_ALGO.startswith("RS"):
if self.OIDC_RP_IDP_SIGN_KEY is not None:
key = self.OIDC_RP_IDP_SIGN_KEY
if self.settings["OIDC_RP_SIGN_ALGO"].startswith("RS"):
if self.settings["OIDC_RP_IDP_SIGN_KEY"] is not None:
key = self.settings["OIDC_RP_IDP_SIGN_KEY"]
else:
key = self.retrieve_matching_jwk(token)
else:
key = self.OIDC_RP_CLIENT_SECRET
key = self.settings["OIDC_RP_CLIENT_SECRET"]

payload_data = self.get_payload_data(token, key)

Expand Down Expand Up @@ -228,7 +235,7 @@ def get_token(self, payload):
del payload["client_secret"]

response = requests.post(
self.OIDC_OP_TOKEN_ENDPOINT,
self.settings["OIDC_OP_TOKEN_ENDPOINT"],
data=payload,
auth=auth,
verify=self.get_settings("OIDC_VERIFY_SSL", True),
Expand All @@ -243,7 +250,7 @@ def get_userinfo(self, access_token, id_token, payload):
the default implementation, but may be used when overriding this method"""

user_response = requests.get(
self.OIDC_OP_USER_ENDPOINT,
self.settings["OIDC_OP_USER_ENDPOINT"],
headers={"Authorization": "Bearer {0}".format(access_token)},
verify=self.get_settings("OIDC_VERIFY_SSL", True),
timeout=self.get_settings("OIDC_TIMEOUT", None),
Expand Down Expand Up @@ -272,8 +279,8 @@ def authenticate(self, request, **kwargs):
)

token_payload = {
"client_id": self.OIDC_RP_CLIENT_ID,
"client_secret": self.OIDC_RP_CLIENT_SECRET,
"client_id": self.settings["OIDC_RP_CLIENT_ID"],
"client_secret": self.settings["OIDC_RP_CLIENT_SECRET"],
"grant_type": "authorization_code",
"code": code,
"redirect_uri": absolutify(self.request, reverse(reverse_url)),
Expand Down
34 changes: 9 additions & 25 deletions mozilla_django_oidc/middleware.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,22 +29,6 @@ class SessionRefresh(MiddlewareMixin):

"""

def __init__(self, get_response):
super(SessionRefresh, self).__init__(get_response)
self.OIDC_EXEMPT_URLS = self.get_settings("OIDC_EXEMPT_URLS", [])
self.OIDC_OP_AUTHORIZATION_ENDPOINT = self.get_settings(
"OIDC_OP_AUTHORIZATION_ENDPOINT"
)
self.OIDC_RP_CLIENT_ID = self.get_settings("OIDC_RP_CLIENT_ID")
self.OIDC_STATE_SIZE = self.get_settings("OIDC_STATE_SIZE", 32)
self.OIDC_AUTHENTICATION_CALLBACK_URL = self.get_settings(
"OIDC_AUTHENTICATION_CALLBACK_URL",
"oidc_authentication_callback",
)
self.OIDC_RP_SCOPES = self.get_settings("OIDC_RP_SCOPES", "openid email")
self.OIDC_USE_NONCE = self.get_settings("OIDC_USE_NONCE", True)
self.OIDC_NONCE_SIZE = self.get_settings("OIDC_NONCE_SIZE", 32)

@staticmethod
def get_settings(attr, *args):
return import_from_settings(attr, *args)
Expand All @@ -61,7 +45,7 @@ def exempt_urls(self):

"""
exempt_urls = []
for url in self.OIDC_EXEMPT_URLS:
for url in self.get_settings("OIDC_EXEMPT_URLS", []):
if not isinstance(url, re_Pattern):
exempt_urls.append(url)
exempt_urls.extend(
Expand All @@ -87,7 +71,7 @@ def exempt_url_patterns(self):
``re.compile(r"/user/[0-9]+/image")``)
"""
exempt_patterns = set()
for url_pattern in self.OIDC_EXEMPT_URLS:
for url_pattern in self.get_settings("OIDC_EXEMPT_URLS", []):
if isinstance(url_pattern, re_Pattern):
exempt_patterns.add(url_pattern)
return exempt_patterns
Expand Down Expand Up @@ -129,27 +113,27 @@ def process_request(self, request):

LOGGER.debug("id token has expired")
# The id_token has expired, so we have to re-authenticate silently.
auth_url = self.OIDC_OP_AUTHORIZATION_ENDPOINT
client_id = self.OIDC_RP_CLIENT_ID
state = get_random_string(self.OIDC_STATE_SIZE)
auth_url = self.get_settings("OIDC_OP_AUTHORIZATION_ENDPOINT")
client_id = self.get_settings("OIDC_RP_CLIENT_ID")
state = get_random_string(self.get_settings("OIDC_STATE_SIZE", 32))

# Build the parameters as if we were doing a real auth handoff, except
# we also include prompt=none.
params = {
"response_type": "code",
"client_id": client_id,
"redirect_uri": absolutify(
request, reverse(self.OIDC_AUTHENTICATION_CALLBACK_URL)
request, reverse(self.get_settings("OIDC_AUTHENTICATION_CALLBACK_URL", "oidc_authentication_callback"))
),
"state": state,
"scope": self.OIDC_RP_SCOPES,
"scope": self.get_settings("OIDC_RP_SCOPES", "openid email"),
"prompt": "none",
}

params.update(self.get_settings("OIDC_AUTH_REQUEST_EXTRA_PARAMS", {}))

if self.OIDC_USE_NONCE:
nonce = get_random_string(self.OIDC_NONCE_SIZE)
if self.get_settings("OIDC_USE_NONCE", True):
nonce = get_random_string(self.get_settings("OIDC_NONCE_SIZE", 32))
params.update({"nonce": nonce})

add_state_and_verifier_and_nonce_to_session(request, state, params)
Expand Down
10 changes: 2 additions & 8 deletions mozilla_django_oidc/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,12 +165,6 @@ class OIDCAuthenticationRequestView(View):

http_method_names = ["get"]

def __init__(self, *args, **kwargs):
super(OIDCAuthenticationRequestView, self).__init__(*args, **kwargs)

self.OIDC_OP_AUTH_ENDPOINT = self.get_settings("OIDC_OP_AUTHORIZATION_ENDPOINT")
self.OIDC_RP_CLIENT_ID = self.get_settings("OIDC_RP_CLIENT_ID")

@staticmethod
def get_settings(attr, *args):
return import_from_settings(attr, *args)
Expand All @@ -186,7 +180,7 @@ def get(self, request):
params = {
"response_type": "code",
"scope": self.get_settings("OIDC_RP_SCOPES", "openid email"),
"client_id": self.OIDC_RP_CLIENT_ID,
"client_id": self.get_settings("OIDC_RP_CLIENT_ID"),
"redirect_uri": absolutify(request, reverse(reverse_url)),
"state": state,
}
Expand Down Expand Up @@ -232,7 +226,7 @@ def get(self, request):

query = urlencode(params)
redirect_url = "{url}?{query}".format(
url=self.OIDC_OP_AUTH_ENDPOINT, query=query
url=self.get_settings("OIDC_OP_AUTHORIZATION_ENDPOINT"), query=query
)
return HttpResponseRedirect(redirect_url)

Expand Down