Switch the home link to logout on security error pages #1564

leanneeliatra · 2023-08-28T16:16:01Z

Description

Fix for bug [BUG] missing roles button doesn't do anything #1490

Why these changes are required?

The button on the /customerror/missing-role page was not doing anything, it should bring the user back to the dashboard

What is the old behavior before changes and new behavior after changes?

Old behaviour

From the /app/customerror/missing-role screen a button 'Back to OpenSearch dashboards Home ...' did nothing when clicked.

New behaviour

From the /app/customerror/missing-role screen a button 'Back to OpenSearch dashboards Home ...' routes to the app dashboard when clicked

Issues Resolved

[BUG] missing roles button doesn't do anything #1490

Testing

complete
Cypress integration tests added
Unit tests not required, signed off by @peternied
Rigorous manual testing locally complete
[Please provide details of testing done: unit testing, integration testing and manual testing]

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

codecov · 2023-08-28T16:27:31Z

Codecov Report

Merging #1564 (b4d16db) into main (d97192d) will increase coverage by 0.05%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##             main    #1564      +/-   ##
==========================================
+ Coverage   67.93%   67.99%   +0.05%     
==========================================
  Files          93       93              
  Lines        2339     2340       +1     
  Branches      317      317              
==========================================
+ Hits         1589     1591       +2     
+ Misses        722      720       -2     
- Partials       28       29       +1

Files	Coverage Δ
public/apps/customerror/custom-error.tsx	`25.00% <0.00%> (+25.00%)`	⬆️

peternied

Thanks for adding this, before we merge could you create/modify a test case that validates the link works?

leanneeliatra · 2023-08-29T09:39:50Z

Thanks for adding this, before we merge could you create/modify a test case that validates the link works?

Yes, for sure Peter thanks. They're in progress I just wanted to submit the changes so far in case there were any comments for the change.

cwperks

Hi @leanneeliatra , from what I understand of the issue the only real action a user with 0 roles could do would be to logout. Reading the comments on the issue, it sounds like the button on that page should be replaced with a logout button. Without a logout button the user would have to clear cookies in the browser to logout.

#1490 (comment)

cwperks · 2023-09-05T13:29:59Z

@leanneeliatra Please see my comment above. From the conversation on the issue, this button should be changed to a logout button.

Hi @leanneeliatra , from what I understand of the issue the only real action a user with 0 roles could do would be to logout. Reading the comments on the issue, it sounds like the button on that page should be replaced with a logout button. Without a logout button the user would have to clear cookies in the browser to logout.

#1490 (comment)

leanneeliatra · 2023-09-06T11:49:25Z

@leanneeliatra Please see my comment above. From the conversation on the issue, this button should be changed to a logout button.

Hi @leanneeliatra , from what I understand of the issue the only real action a user with 0 roles could do would be to logout. Reading the comments on the issue, it sounds like the button on that page should be replaced with a logout button. Without a logout button the user would have to clear cookies in the browser to logout.

#1490 (comment)

Thank you Craig, that's complete

leanneeliatra · 2023-09-08T12:48:21Z

Hi @peternied I have added the integration tests: opensearch-project/opensearch-dashboards-functional-test#830
There are unit tests covering the logout functionality so that's covered from that aspect.
& I changed the functionality to a logout button cc @cwperks.
That should be everything. Thank you all.

public/apps/customerror/custom-error.tsx

derek-ho

Other than button text change looks good. Can we change the base branch to main and backport to 2.x?

derek-ho · 2023-09-08T14:45:17Z

This should also fix: #842

leanneeliatra

I have added the integration tests @peternied
opensearch-project/opensearch-dashboards-functional-test#830

Unfortunately I'm not sure how to address the '1 change requested: Merging can be performed automatically once the requested changes are addressed.'

Could you accept the changes please? If you are happy with the test linked above? Thank you.

peternied

Thanks for adding the tests.

The base branch was changed.

peternied · 2023-09-11T22:40:51Z

I just saw that this PR was pointed to the 2.8 branch, it should go into main, please rebase against main

leanneeliatra · 2023-09-12T10:22:59Z

I just saw that this PR was pointed to the 2.8 branch, it should go into main, please rebase against main

I will indeed thanks @peternied

leanneeliatra · 2023-09-29T16:02:41Z

Hi @leanneeliatra and @peternied . I think the socket failure is related to this issue: cypress-io/cypress-documentation#5483. So it is not from our end. But I can try to fix it by rolling back the chrome driver version.

Thanks for looking into this @RyanL1997 and yes, please do that. It would be great to get this merged in! Thanks.

leanneeliatra · 2023-10-04T07:38:42Z

Update: frontend pipeline should be fixed, could the checks on this MR be re-run please (I do not have access to do that)
cc @peternied

leanneeliatra · 2023-10-05T15:11:51Z

I am pulling in @RyanL1997 updates to the CI that will fix my checks on this PR and which will also simultaneously rerun the checks.

Signed-off-by: [email protected] <[email protected]>

leanneeliatra · 2023-10-09T16:27:20Z

The main branch has been merged in to my own branch but the CI is still failing, I will reach out to @RyanL1997 to discuss/for thoughts.

leanneeliatra · 2023-10-10T08:34:57Z

Myself and @RyanL1997 are in communication about the cypress tests.

leanneeliatra · 2023-10-11T09:23:14Z

@RyanL1997 is going to have a look at the cypress tests for me (thank you :) )

RyanL1997 · 2023-10-12T01:36:43Z

Reference to #1599, the manual testing results:

leanneeliatra · 2023-10-12T09:17:41Z

Thanks a million @RyanL1997 I really appreciate that!

No further reviews are needed.

leanneeliatra · 2023-10-13T15:03:35Z

Can this be merged please?

peternied · 2023-10-13T17:37:44Z

@RyanL1997 could you look into the CI issue?

leanneeliatra · 2023-10-23T12:11:08Z

Can this be merged please! :) @peternied has previously stated (see comment referenced below) he was happy with the coverage so please merge ASAP. Thank you.

leanneeliatra · 2023-10-23T12:14:49Z

@leanneeliatra I've re-triggered CI test execution, I saw socket failures which would be completely unrelated to your change. @RyanL1997 could you confirm the state of the cypress tests?

Don't worry about that code coverage issue - I am not concerned about that line getting coverage.

Please merge ASAP. Thank you.

* changing return to dashboards with functioning logout button on error page Signed-off-by: [email protected] <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Co-authored-by: Peter Nied <[email protected]> (cherry picked from commit 27c37d4)

* changing return to dashboards with functioning logout button on error page Signed-off-by: [email protected] <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Co-authored-by: Peter Nied <[email protected]> (cherry picked from commit 27c37d4) Co-authored-by: leanneeliatra <[email protected]>

…oject#1564) * changing return to dashboards with functioning logout button on error page Signed-off-by: [email protected] <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Co-authored-by: Peter Nied <[email protected]> Signed-off-by: [email protected] <[email protected]>

leanneeliatra force-pushed the 2.8 branch from d0a1867 to ddd55eb Compare August 28, 2023 16:19

peternied requested changes Aug 28, 2023

View reviewed changes

cwperks reviewed Aug 29, 2023

View reviewed changes

leanneeliatra requested a review from cwperks September 5, 2023 07:12

leanneeliatra mentioned this pull request Sep 7, 2023

When a user has no roles, logout on button click opensearch-project/opensearch-dashboards-functional-test#830

Merged

1 task

leanneeliatra force-pushed the 2.8 branch from 6e8fd19 to 25d927a Compare September 8, 2023 12:43

leanneeliatra mentioned this pull request Sep 8, 2023

[BUG] missing roles button doesn't do anything #1490

Closed

leanneeliatra requested a review from peternied September 8, 2023 12:47

derek-ho reviewed Sep 8, 2023

View reviewed changes

public/apps/customerror/custom-error.tsx Outdated Show resolved Hide resolved

derek-ho approved these changes Sep 8, 2023

View reviewed changes

leanneeliatra commented Sep 8, 2023

View reviewed changes

cwperks previously approved these changes Sep 8, 2023

View reviewed changes

peternied previously approved these changes Sep 11, 2023

View reviewed changes

peternied changed the title ~~Rerouting to app/home on button click~~ Switch the home link to logout on security error pages Sep 11, 2023

peternied changed the base branch from 2.8 to main September 11, 2023 22:39

peternied requested review from cliu123, DarshitChanpura, davidlago, RyanL1997 and stephen-crawford as code owners September 11, 2023 22:39

DarshitChanpura approved these changes Sep 29, 2023

View reviewed changes

Merge branch 'main' into 2.8

ab94b85

peternied and others added 3 commits October 6, 2023 13:33

Merge branch 'main' into 2.8

382bd8c

trigger ci

c21f06e

Signed-off-by: [email protected] <[email protected]>

trigger ci

8f23325

Signed-off-by: [email protected] <[email protected]>

leanneeliatra mentioned this pull request Oct 10, 2023

Tenant change related integration tests - allow tenant change and retain tenant in shortlink opensearch-project/opensearch-dashboards-functional-test#736

Merged

1 task

RyanL1997 approved these changes Oct 12, 2023

View reviewed changes

Merge branch 'main' into 2.8

9002a63

Merge branch 'main' into 2.8

b4d16db

cwperks approved these changes Oct 23, 2023

View reviewed changes

cwperks added the backport 2.x backport to 2.x branch label Oct 23, 2023

cwperks merged commit 27c37d4 into opensearch-project:main Oct 23, 2023
9 of 10 checks passed

opensearch-trigger-bot bot mentioned this pull request Oct 23, 2023

[Backport 2.x] Switch the home link to logout on security error pages #1629

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch the home link to logout on security error pages #1564

Switch the home link to logout on security error pages #1564

leanneeliatra commented Aug 28, 2023 •

edited

Loading

codecov bot commented Aug 28, 2023 •

edited

Loading

peternied left a comment

leanneeliatra commented Aug 29, 2023

cwperks left a comment •

edited

Loading

cwperks commented Sep 5, 2023

leanneeliatra commented Sep 6, 2023

leanneeliatra commented Sep 8, 2023 •

edited

Loading

derek-ho left a comment

derek-ho commented Sep 8, 2023

leanneeliatra left a comment •

edited

Loading

peternied left a comment

peternied commented Sep 11, 2023

leanneeliatra commented Sep 12, 2023

leanneeliatra commented Sep 29, 2023

leanneeliatra commented Oct 4, 2023 •

edited

Loading

leanneeliatra commented Oct 5, 2023 •

edited

Loading

leanneeliatra commented Oct 9, 2023 •

edited

Loading

leanneeliatra commented Oct 10, 2023 •

edited

Loading

leanneeliatra commented Oct 11, 2023

RyanL1997 commented Oct 12, 2023 •

edited

Loading

leanneeliatra commented Oct 12, 2023 •

edited

Loading

leanneeliatra commented Oct 13, 2023

peternied commented Oct 13, 2023

leanneeliatra commented Oct 23, 2023 •

edited

Loading

leanneeliatra commented Oct 23, 2023

Switch the home link to logout on security error pages #1564

Switch the home link to logout on security error pages #1564

Conversation

leanneeliatra commented Aug 28, 2023 • edited Loading

Description

Category

Why these changes are required?

What is the old behavior before changes and new behavior after changes?

Issues Resolved

Testing

Check List

codecov bot commented Aug 28, 2023 • edited Loading

Codecov Report

peternied left a comment

Choose a reason for hiding this comment

leanneeliatra commented Aug 29, 2023

cwperks left a comment • edited Loading

Choose a reason for hiding this comment

cwperks commented Sep 5, 2023

leanneeliatra commented Sep 6, 2023

leanneeliatra commented Sep 8, 2023 • edited Loading

derek-ho left a comment

Choose a reason for hiding this comment

derek-ho commented Sep 8, 2023

leanneeliatra left a comment • edited Loading

Choose a reason for hiding this comment

peternied left a comment

Choose a reason for hiding this comment

peternied commented Sep 11, 2023

leanneeliatra commented Sep 12, 2023

leanneeliatra commented Sep 29, 2023

leanneeliatra commented Oct 4, 2023 • edited Loading

leanneeliatra commented Oct 5, 2023 • edited Loading

leanneeliatra commented Oct 9, 2023 • edited Loading

leanneeliatra commented Oct 10, 2023 • edited Loading

leanneeliatra commented Oct 11, 2023

RyanL1997 commented Oct 12, 2023 • edited Loading

leanneeliatra commented Oct 12, 2023 • edited Loading

leanneeliatra commented Oct 13, 2023

peternied commented Oct 13, 2023

leanneeliatra commented Oct 23, 2023 • edited Loading

leanneeliatra commented Oct 23, 2023

leanneeliatra commented Aug 28, 2023 •

edited

Loading

codecov bot commented Aug 28, 2023 •

edited

Loading

cwperks left a comment •

edited

Loading

leanneeliatra commented Sep 8, 2023 •

edited

Loading

leanneeliatra left a comment •

edited

Loading

leanneeliatra commented Oct 4, 2023 •

edited

Loading

leanneeliatra commented Oct 5, 2023 •

edited

Loading

leanneeliatra commented Oct 9, 2023 •

edited

Loading

leanneeliatra commented Oct 10, 2023 •

edited

Loading

RyanL1997 commented Oct 12, 2023 •

edited

Loading

leanneeliatra commented Oct 12, 2023 •

edited

Loading

leanneeliatra commented Oct 23, 2023 •

edited

Loading