Skip to content
This repository was archived by the owner on Mar 10, 2023. It is now read-only.

Check cookie exists and subject on cookie before using #624

Merged
merged 1 commit into from
May 11, 2020

Conversation

Waterdrips
Copy link
Contributor

@Waterdrips Waterdrips commented Mar 26, 2020

Description

We were checking if we had a cookie, but not then checking if it was not
empty, and the subject was not empty before using it for redirecting to
the user's dashboard (If they navigated to / or /dashboard)

Fixes #623

How Has This Been Tested?

Tested by deploying new dashboard, deleting cookie, setting cookie to
empty string etc. All returned no error (but did show 401 not
authorized)

Signed-off-by: Alistair Hey [email protected]

How are existing users impacted? What migration steps/scripts do we need?

This is an edge-case which i was not able to replicate. New deployment of the dashboard to get the latest fix.

Checklist:

I have:

  • updated the documentation and/or roadmap (if required)
  • read the CONTRIBUTION guide
  • signed-off my commits with git commit -s
  • added unit tests

We were checking if we had a cookie, but not then checking if it was not
empty, and the subject was not empty before using it for redirecting to
the user's dashboard (If they navigated to / or /dashboard)

Tested by deploying new dashboard, deleting cookie, setting cookie to
empty string etc. All returned no error (but did show 401 not
authorized)

Signed-off-by: Alistair Hey <[email protected]>
@Waterdrips Waterdrips mentioned this pull request Apr 18, 2020
5 tasks
Waterdrips added a commit to Waterdrips/ofc-bootstrap that referenced this pull request Apr 26, 2020
The env var for cookie_root_domain was missing from the system_dashboard
deployment, this meant when users with OAuth enabled did not get their
token removed when clicking logout.

see openfaas/openfaas-cloud#623
see openfaas/openfaas-cloud#624

This was tested by updating an existing deployment with the new setting
and logging out, the cookie was removed and the oauth flow was triggered
to gain a new token

Signed-off-by: Alistair Hey <[email protected]>
Copy link
Member

@alexellis alexellis left a comment

Choose a reason for hiding this comment

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

Approved

@alexellis alexellis merged commit be0955b into openfaas:master May 11, 2020
alexellis pushed a commit to openfaas/ofc-bootstrap that referenced this pull request May 11, 2020
The env var for cookie_root_domain was missing from the system_dashboard
deployment, this meant when users with OAuth enabled did not get their
token removed when clicking logout.

see openfaas/openfaas-cloud#623
see openfaas/openfaas-cloud#624

This was tested by updating an existing deployment with the new setting
and logging out, the cookie was removed and the oauth flow was triggered
to gain a new token

Signed-off-by: Alistair Hey <[email protected]>
@Waterdrips Waterdrips deleted the check-existance-of-cookie branch July 16, 2020 06:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid cooke causes dashboard to fail on load.
2 participants