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.

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

Allow both 400 and 401 for JWT expiry responses #2098

Merged
merged 2 commits into from
Jun 17, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
5 changes: 3 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ This project adheres to [Semantic Versioning](https://semver.org/).

### Fixed

- [#2098](https://github.com/plotly/dash/pull/2098) Accept HTTP code 400 as well as 401 for JWT expiry
- [#2097](https://github.com/plotly/dash/pull/2097) Fix bug [#2095](https://github.com/plotly/dash/issues/2095) with TypeScript compiler and `React.FC` empty valueDeclaration error & support empty props components.

## [2.5.1] - 2022-06-13
Expand Down Expand Up @@ -35,8 +36,8 @@ This feature can be disabled by providing an empty viewport meta tag. e.g. `app

### Fixed

- [#2043](https://github.com/plotly/dash/pull/2043) Fix bug
[#2003](https://github.com/plotly/dash/issues/2003) in which
- [#2043](https://github.com/plotly/dash/pull/2043) Fix bug
[#2003](https://github.com/plotly/dash/issues/2003) in which
`dangerously_allow_html=True` + `mathjax=True` works in some cases, and in some cases not.
- [#2065](https://github.com/plotly/dash/pull/2065) Fix bug [#2064](https://github.com/plotly/dash/issues/2064) rendering of `dcc.Dropdown` with a value but no options.
- [#2047](https://github.com/plotly/dash/pull/2047) Fix bug [#1979](https://github.com/plotly/dash/issues/1979) in which `DASH_DEBUG` as environment variable gets ignored.
Expand Down
5 changes: 4 additions & 1 deletion dash/dash-renderer/src/actions/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,10 @@ export default function apiThunk(endpoint, method, store, id, body) {
return;
}

if (res.status === STATUS.UNAUTHORIZED) {
if (
res.status === STATUS.UNAUTHORIZED ||
res.status === STATUS.BAD_REQUEST
) {
if (hooks.request_refresh_jwt) {
const body = await res.text();
if (body.includes(JWT_EXPIRED_MESSAGE)) {
Expand Down
3 changes: 2 additions & 1 deletion dash/dash-renderer/src/actions/callbacks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -536,7 +536,8 @@ export function executeCallback(
lastError = res;
if (
retry <= MAX_AUTH_RETRIES &&
res.status === STATUS.UNAUTHORIZED
(res.status === STATUS.UNAUTHORIZED ||
res.status === STATUS.BAD_REQUEST)
) {
const body = await res.text();

Expand Down
4 changes: 4 additions & 0 deletions dash/dash-renderer/src/constants/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ export const JWT_EXPIRED_MESSAGE = 'JWT Expired';
export const STATUS = {
OK: 200,
PREVENT_UPDATE: 204,
// We accept both 400 and 401 for JWT token expiry responses.
// Some servers use code 400 for expired tokens, because
// they reserve 401 for cases that require user action
BAD_REQUEST: 400,
UNAUTHORIZED: 401,
CLIENTSIDE_ERROR: 'CLIENTSIDE_ERROR',
NO_RESPONSE: 'NO_RESPONSE'
Expand Down
2 changes: 1 addition & 1 deletion dash/development/base_component.py
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ def _set_random_id(self):
"""
)

v = str(uuid.UUID(int=rd.randint(0, 2 ** 128)))
v = str(uuid.UUID(int=rd.randint(0, 2**128)))
Copy link
Contributor

Choose a reason for hiding this comment

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

🙈

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will have to look into whether we're linting correctly on CI - this was caught by the precommit hook for me 🤷

setattr(self, "id", v)
return v

Expand Down
6 changes: 4 additions & 2 deletions tests/integration/renderer/test_request_hooks.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import json
import functools
import flask
import pytest

from dash import Dash, Output, Input, html, dcc
from werkzeug.exceptions import HTTPException
Expand Down Expand Up @@ -195,7 +196,8 @@ def update_output(value):
assert dash_duo.get_logs() == []


def test_rdrh003_refresh_jwt(dash_duo):
@pytest.mark.parametrize("expiry_code", [401, 400])
def test_rdrh003_refresh_jwt(expiry_code, dash_duo):

app = Dash(__name__)

Expand Down Expand Up @@ -260,7 +262,7 @@ def wrap(*args, **kwargs):
):
# Read the data to prevent bug with base http server.
flask.request.get_json(silent=True)
flask.abort(401, description="JWT Expired " + str(token))
flask.abort(expiry_code, description="JWT Expired " + str(token))
except HTTPException as e:
return e
return func(*args, **kwargs)
Expand Down