зеркало из https://github.com/mozilla/treeherder.git
Bug 1529223 - Auth: Use seconds not milliseconds for expiration handling
Previously the frontend would calculate the access token expiry timestamp in milliseconds and pass it to the `/login/` API via an `ExpiresAt` header. The backend would then convert both the Id Token's `exp` and current time to milliseconds, when calculating the earliest expiry. The result then had to be converted back to seconds for use with Django's session `.set_expiry()`. It is instead much simpler to leave everything in seconds, since none of the Auth0-provided inputs were in milliseconds to start with, so there is no loss of precision, just fewer conversions required. Timestamps are also more commonly in seconds, so use of seconds is less surprising. After this is deployed there will initially be users who have old frontend pages open that are still sending the expiry as milliseconds. In order to be able to differentiate between new and old clients, the header has been renamed to `Access-Token-Expires-At` (which also makes it clearer as to what the expiry is for, given there is also an Id Token expiry), and a temporary fall-back added to the backend that can be removed after a few days has passed.
This commit is contained in:
Ed Morley
Родитель
6c05a78eb4
Коммит
e643b8e4e7
|
|
@ -62,6 +62,7 @@ def test_login_logout_relogin(client, monkeypatch, id_token_sub, id_token_email,
|
|||
"""
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
access_token_expiration_timestamp = now_in_seconds + one_hour_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': id_token_sub, 'email': id_token_email, 'exp': id_token_expiration_timestamp}
|
||||
|
|
@ -71,10 +72,6 @@ def test_login_logout_relogin(client, monkeypatch, id_token_sub, id_token_email,
|
|||
assert auth_session_key not in client.session
|
||||
assert User.objects.count() == 0
|
||||
|
||||
# Confusingly the `ExpiresAt` header is expected to be in milliseconds.
|
||||
# TODO: Change the frontend to pass seconds instead.
|
||||
expires_at = (now_in_seconds + one_hour_in_seconds) * 1000
|
||||
|
||||
# The first time someone logs in a new user should be created,
|
||||
# which is then associated with their Django session.
|
||||
|
||||
|
|
@ -82,7 +79,7 @@ def test_login_logout_relogin(client, monkeypatch, id_token_sub, id_token_email,
|
|||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer meh',
|
||||
HTTP_IDTOKEN='meh',
|
||||
HTTP_EXPIRESAT=str(expires_at)
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp)
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {
|
||||
|
|
@ -113,7 +110,7 @@ def test_login_logout_relogin(client, monkeypatch, id_token_sub, id_token_email,
|
|||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer meh',
|
||||
HTTP_IDTOKEN='meh',
|
||||
HTTP_EXPIRESAT=str(expires_at)
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp)
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()['username'] == expected_username
|
||||
|
|
@ -130,21 +127,18 @@ def test_login_same_email_different_provider(test_ldap_user, client, monkeypatch
|
|||
"""
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
access_token_expiration_timestamp = now_in_seconds + one_hour_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'email', 'email': test_ldap_user.email, 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
# Confusingly the `ExpiresAt` header is expected to be in milliseconds.
|
||||
# TODO: Change the frontend to pass seconds instead.
|
||||
expires_at = (now_in_seconds + one_hour_in_seconds) * 1000
|
||||
|
||||
resp = client.get(
|
||||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer meh',
|
||||
HTTP_IDTOKEN='meh',
|
||||
HTTP_EXPIRESAT=str(expires_at)
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp)
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()['username'] == 'email/user@foo.com'
|
||||
|
|
@ -155,21 +149,18 @@ def test_login_unknown_identity_provider(client, monkeypatch):
|
|||
"""Test an id token `sub` value that does not match a known identity provider."""
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
access_token_expiration_timestamp = now_in_seconds + one_hour_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'bad', 'email': 'foo@bar.com', 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
# Confusingly the `ExpiresAt` header is expected to be in milliseconds.
|
||||
# TODO: Change the frontend to pass seconds instead.
|
||||
expires_at = (now_in_seconds + one_hour_in_seconds) * 1000
|
||||
|
||||
resp = client.get(
|
||||
reverse("auth-login"),
|
||||
HTTP_AUTHORIZATION="Bearer meh",
|
||||
HTTP_IDTOKEN="meh",
|
||||
HTTP_EXPIRESAT=str(expires_at)
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp)
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()["detail"] == "Unrecognized identity"
|
||||
|
|
@ -180,16 +171,13 @@ def test_login_not_active(test_ldap_user, client, monkeypatch):
|
|||
"""Test that login is not permitted if the user has been disabled."""
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
access_token_expiration_timestamp = now_in_seconds + one_hour_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'Mozilla-LDAP', 'email': test_ldap_user.email, 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
# Confusingly the `ExpiresAt` header is expected to be in milliseconds.
|
||||
# TODO: Change the frontend to pass seconds instead.
|
||||
expires_at = (now_in_seconds + one_hour_in_seconds) * 1000
|
||||
|
||||
test_ldap_user.is_active = False
|
||||
test_ldap_user.save()
|
||||
|
||||
|
|
@ -197,7 +185,7 @@ def test_login_not_active(test_ldap_user, client, monkeypatch):
|
|||
reverse("auth-login"),
|
||||
HTTP_AUTHORIZATION="Bearer meh",
|
||||
HTTP_IDTOKEN="meh",
|
||||
HTTP_EXPIRESAT=str(expires_at)
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp)
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()["detail"] == "This user has been disabled."
|
||||
|
|
@ -309,3 +297,110 @@ def test_login_id_token_invalid_signature(client):
|
|||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()['detail'] == 'Invalid header: Unable to parse authentication'
|
||||
|
||||
|
||||
def test_login_access_token_expiry_header_missing(client, monkeypatch):
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'Mozilla-LDAP', 'email': 'x@y.z', 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
resp = client.get(
|
||||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer foo',
|
||||
HTTP_IDTOKEN='bar',
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()['detail'] == 'Access-Token-Expires-At header is expected'
|
||||
|
||||
|
||||
def test_login_access_token_expiry_header_malformed(client, monkeypatch):
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'Mozilla-LDAP', 'email': 'x@y.z', 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
resp = client.get(
|
||||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer foo',
|
||||
HTTP_IDTOKEN='bar',
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT='aaa',
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()['detail'] == 'Access-Token-Expires-At header value is invalid'
|
||||
|
||||
|
||||
def test_login_access_token_expired(client, monkeypatch):
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_hour_in_seconds
|
||||
access_token_expiration_timestamp = now_in_seconds - 30
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'Mozilla-LDAP', 'email': 'x@y.z', 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
resp = client.get(
|
||||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer foo',
|
||||
HTTP_IDTOKEN='bar',
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp),
|
||||
)
|
||||
assert resp.status_code == 403
|
||||
assert resp.json()['detail'] == 'Session expiry time has already passed!'
|
||||
|
||||
|
||||
def test_login_id_token_expires_before_access_token(test_ldap_user, client, monkeypatch):
|
||||
"""
|
||||
Test that the Django session expiration is set correctly if the Id Token expiration
|
||||
is due to occur before the Access Token expires (normally it is the other way around).
|
||||
"""
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_hour_in_seconds
|
||||
access_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'email', 'email': test_ldap_user.email, 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
resp = client.get(
|
||||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer meh',
|
||||
HTTP_IDTOKEN='meh',
|
||||
HTTP_ACCESS_TOKEN_EXPIRES_AT=str(access_token_expiration_timestamp)
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert client.session.get_expiry_age() == pytest.approx(one_hour_in_seconds, abs=5)
|
||||
|
||||
|
||||
# TODO: Remove once enough time has passed for people to reload open UI tabs.
|
||||
def test_login_legacy_headers(test_ldap_user, client, monkeypatch):
|
||||
"""
|
||||
Test that requests made using the `ExpiresAt` header still succeed.
|
||||
"""
|
||||
now_in_seconds = int(time.time())
|
||||
id_token_expiration_timestamp = now_in_seconds + one_day_in_seconds
|
||||
|
||||
def userinfo_mock(*args, **kwargs):
|
||||
return {'sub': 'Mozilla-LDAP', 'email': test_ldap_user.email, 'exp': id_token_expiration_timestamp}
|
||||
|
||||
monkeypatch.setattr(AuthBackend, '_get_user_info', userinfo_mock)
|
||||
|
||||
expires_at_in_milliseconds = (now_in_seconds + one_hour_in_seconds) * 1000
|
||||
|
||||
resp = client.get(
|
||||
reverse('auth-login'),
|
||||
HTTP_AUTHORIZATION='Bearer abc',
|
||||
HTTP_IDTOKEN='abc',
|
||||
HTTP_EXPIRESAT=str(expires_at_in_milliseconds)
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()['username'] == test_ldap_user.username
|
||||
assert client.session.get_expiry_age() == pytest.approx(one_hour_in_seconds, abs=5)
|
||||
|
|
|
|||
|
|
@ -32,13 +32,21 @@ with open('treeherder/auth/jwks.json') as f:
|
|||
|
||||
class AuthBackend(object):
|
||||
|
||||
def _get_accesstoken_expiry(self, request):
|
||||
expires_at_in_milliseconds = request.META.get("HTTP_EXPIRESAT")
|
||||
def _get_access_token_expiry(self, request):
|
||||
expiration_timestamp_in_seconds = request.META.get('HTTP_ACCESS_TOKEN_EXPIRES_AT')
|
||||
|
||||
if not expires_at_in_milliseconds:
|
||||
raise AuthenticationFailed("expiresAt header is expected")
|
||||
if not expiration_timestamp_in_seconds:
|
||||
# Check for the previous name/type for this header.
|
||||
# TODO: Remove once enough time has passed for people to reload open UI tabs.
|
||||
expiration_timestamp_in_milliseconds = request.META.get('HTTP_EXPIRESAT')
|
||||
if not expiration_timestamp_in_milliseconds:
|
||||
raise AuthenticationFailed('Access-Token-Expires-At header is expected')
|
||||
expiration_timestamp_in_seconds = int(expiration_timestamp_in_milliseconds) / 1000
|
||||
|
||||
return int(expires_at_in_milliseconds)
|
||||
try:
|
||||
return int(expiration_timestamp_in_seconds)
|
||||
except ValueError:
|
||||
raise AuthenticationFailed('Access-Token-Expires-At header value is invalid')
|
||||
|
||||
def _get_access_token(self, request):
|
||||
auth = request.META.get('HTTP_AUTHORIZATION')
|
||||
|
|
@ -166,20 +174,23 @@ class AuthBackend(object):
|
|||
user_info = self._get_user_info(access_token, id_token)
|
||||
username = self._get_username_from_userinfo(user_info)
|
||||
|
||||
accesstoken_exp_in_ms = self._get_accesstoken_expiry(request)
|
||||
access_token_expiry_timestamp = self._get_access_token_expiry(request)
|
||||
# Per http://openid.net/specs/openid-connect-core-1_0.html#IDToken, exp is given in seconds
|
||||
idtoken_exp_in_ms = user_info['exp'] * 1000
|
||||
now_in_ms = int(round(time.time() * 1000))
|
||||
id_token_expiry_timestamp = user_info['exp']
|
||||
now_in_seconds = int(time.time())
|
||||
|
||||
# The default Django user session length is overridden, since otherwise the access token
|
||||
# in localstorage in the UI (used for Taskcluster interactions) might expire before the
|
||||
# Django session does, resulting in confusing UX.
|
||||
# The session length is set to match whichever token expiration time is closer.
|
||||
session_expiry_in_ms = min(accesstoken_exp_in_ms, idtoken_exp_in_ms)
|
||||
expires_in = int((session_expiry_in_ms - now_in_ms) / 1000)
|
||||
earliest_expiration_timestamp = min(access_token_expiry_timestamp, id_token_expiry_timestamp)
|
||||
seconds_until_expiry = earliest_expiration_timestamp - now_in_seconds
|
||||
|
||||
logger.debug('Updating session to expire in %i seconds', expires_in)
|
||||
request.session.set_expiry(expires_in)
|
||||
if seconds_until_expiry <= 0:
|
||||
raise AuthError('Session expiry time has already passed!')
|
||||
|
||||
logger.debug('Updating session to expire in %i seconds', seconds_until_expiry)
|
||||
request.session.set_expiry(seconds_until_expiry)
|
||||
|
||||
try:
|
||||
return User.objects.get(username=username)
|
||||
|
|
|
|||
|
|
@ -15,7 +15,43 @@ export const webAuth = new WebAuth({
|
|||
});
|
||||
|
||||
export const userSessionFromAuthResult = authResult => {
|
||||
const expiresAt = JSON.stringify(authResult.expiresIn * 1000 + Date.now());
|
||||
// Example authResult:
|
||||
// {
|
||||
// "accessToken": "<TOKEN>",
|
||||
// "idToken": "<TOKEN>",
|
||||
// "idTokenPayload": {
|
||||
// "https://sso.mozilla.com/claim/groups": [
|
||||
// "all_scm_level_1",
|
||||
// "all_scm_level_2",
|
||||
// "all_scm_level_3"
|
||||
// ],
|
||||
// "given_name": "Firstname",
|
||||
// "family_name": "Surname",
|
||||
// "nickname": "Firstname Surname",
|
||||
// "name": "Firstname Surname",
|
||||
// "picture": "<GRAVATAR_URL>",
|
||||
// "updated_at": "2019-02-13T17:26:19.538Z",
|
||||
// "email": "fsurname@mozilla.com",
|
||||
// "email_verified": true,
|
||||
// "iss": "https://auth.mozilla.auth0.com/",
|
||||
// "sub": "ad|Mozilla-LDAP|fsurname",
|
||||
// "aud": "<HASH>",
|
||||
// "iat": 1550078779,
|
||||
// "exp": 1550683579,
|
||||
// "at_hash": "<HASH>",
|
||||
// "nonce": "<HASH>"
|
||||
// },
|
||||
// "appState": null,
|
||||
// "refreshToken": null,
|
||||
// "state": "<HASH>",
|
||||
// "expiresIn": 86400,
|
||||
// "tokenType": "Bearer",
|
||||
// "scope": "openid profile email taskcluster-credentials"
|
||||
// }
|
||||
//
|
||||
// For more details, see:
|
||||
// https://auth0.com/docs/libraries/auth0js/v9#extract-the-authresult-and-get-user-info
|
||||
//
|
||||
const userSession = {
|
||||
idToken: authResult.idToken,
|
||||
accessToken: authResult.accessToken,
|
||||
|
|
@ -23,8 +59,9 @@ export const userSessionFromAuthResult = authResult => {
|
|||
picture: authResult.idTokenPayload.picture,
|
||||
oidcSubject: authResult.idTokenPayload.sub,
|
||||
url: authResult.url,
|
||||
// expiresAt is used by the django backend to expire the user session
|
||||
expiresAt,
|
||||
// `accessTokenexpiresAt` is the unix timestamp (in seconds) at which the access token expires.
|
||||
// It is used by the Django backend along with idToken's `exp` to determine session expiry.
|
||||
accessTokenExpiresAt: authResult.expiresIn + Math.floor(Date.now() / 1000),
|
||||
// per https://wiki.mozilla.org/Security/Guidelines/OpenID_connect#Session_handling
|
||||
renewAfter: fromNow('15 minutes'),
|
||||
};
|
||||
|
|
|
|||
|
|
@ -18,9 +18,9 @@ export default class AuthService {
|
|||
return new Promise(async (resolve, reject) => {
|
||||
const userResponse = await fetch(loginUrl, {
|
||||
headers: {
|
||||
authorization: `Bearer ${userSession.accessToken}`,
|
||||
Authorization: `Bearer ${userSession.accessToken}`,
|
||||
'Access-Token-Expires-At': userSession.accessTokenExpiresAt,
|
||||
idToken: userSession.idToken,
|
||||
expiresAt: userSession.expiresAt,
|
||||
},
|
||||
method: 'GET',
|
||||
credentials: 'same-origin',
|
||||
|
|
|
|||
Загрузка…
Ссылка в новой задаче