Skip to content

Dash-auth with dds 2.6#75

Merged
T4rk1n merged 12 commits intomasterfrom
dds-2-6
Dec 4, 2018
Merged

Dash-auth with dds 2.6#75
T4rk1n merged 12 commits intomasterfrom
dds-2-6

Conversation

@T4rk1n
Copy link
Copy Markdown
Contributor

@T4rk1n T4rk1n commented Nov 20, 2018

  • Disable index overwrite and route wrapping if DASH_LOGOUT_URL environment variable is set.
  • PlotlyAuth.create_logout_button returns a dcc.LogoutButton with the DASH_LOGOUT_URL if set.

Tests are still failing with timeouts.

  • test with 2.6

@T4rk1n
Copy link
Copy Markdown
Contributor Author

T4rk1n commented Nov 27, 2018

@scjody I tested with on-prem 2.6, the login/logout works but there is no redirect after the logout, the route just just return {}

@scjody
Copy link
Copy Markdown

scjody commented Nov 27, 2018

@T4rk1n Thanks for the testing. I took a look at https://github.com/plotly/dash-deployment-server/pull/190 and it doesn't look like the redirect was added.

@tarzzz Can you please look into this? (Based on the design in https://github.com/plotly/streambed/issues/11817#issuecomment-439107597, a redirect is required so that dash-auth and dcc can send the user to the logout endpoint via a standard form POST.)

@T4rk1n
Copy link
Copy Markdown
Contributor Author

T4rk1n commented Nov 27, 2018

@tarzzz If the redirect could take a url supplied in the form values it would be great. I could take it as prop and set hidden value input.

@tarzzz
Copy link
Copy Markdown

tarzzz commented Nov 27, 2018

@T4rk1n Would prefer to keep it simple at this point, to redirect to the DDS main page. It also makes sense because logging out from an app logs a user out from the DDS as well as all the other apps.

We may add the redirect if it feels necessary.

@T4rk1n
Copy link
Copy Markdown
Contributor Author

T4rk1n commented Nov 28, 2018

Getting 405 on the revoke_token route for the old logout:

Invalidation failure HTTPError(u'405 Client Error: Method Not Allowed for url: https://plot.ly/Auth/o/revoke_token/',)

@T4rk1n
Copy link
Copy Markdown
Contributor Author

T4rk1n commented Dec 3, 2018

@scjody Please review.

We decided at the last dash meeting to deprecate PlotlyAuth and split dash-auth into 3 repos.

  • dash-enterprise-auth for dds auth integrations, DDS 3.0.0 integrations dash-enterprise-auth#1, basically the logout button, get_username and the kerberos ticket method for now.
  • basic_auth.py -> dash-basic-auth
  • oauth.py -> dash-oauth

I added a pending deprecation warning to PlotlyAuth.__init__. I think dopsa should still use dash-auth with this PR patch to disable it if the logout_url is set to ease the transition.

The tests on circleci all get stuck with no output, they still pass locally.

@T4rk1n T4rk1n changed the title [WIP] Dash-auth with dds 2.6 Dash-auth with dds 2.6 Dec 3, 2018
Copy link
Copy Markdown

@scjody scjody left a comment

Choose a reason for hiding this comment

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

Looks good to me!

💃 if this works on both old (2.5.x) and new (2.6 prerelease) versions of DDS.

Comment thread dash_auth/plotly_auth.py Outdated

deprecation_notice = '''
PlotlyAuth is being deprecated.
If your app is still using dash-deployment-server < 2.6,
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

minor, but the product is called Dash Deployment Server (it's not a Python package).

Comment thread dev-requirements.txt Outdated
@@ -1,4 +1,5 @@
dash_core_components
# dash_core_components
-e git+https://github.com/plotly/dash-core-components.git@logout-btn#egg=dash_core_components
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reminder to change this before merging if possible

@T4rk1n
Copy link
Copy Markdown
Contributor Author

T4rk1n commented Dec 4, 2018

Tested on 2.5 and 2.6, both works.

@T4rk1n T4rk1n merged commit 251aeef into master Dec 4, 2018
@T4rk1n T4rk1n deleted the dds-2-6 branch December 4, 2018 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants