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

Run security-dashboards-plugin CI while security-dashboards-plugin is split into 2 separate plugins #7

Open
wants to merge 47 commits into
base: main
Choose a base branch
from

Conversation

cwperks
Copy link
Owner

@cwperks cwperks commented May 22, 2024

Companion PR: https://github.com/cwperks/OpenSearch-Dashboards/pull/1/files

In this PR, the security-dashboards-plugin is installed twice for cypress testing.

  1. Session Management Only Mode - In the first instance, the security admin pages are removed and security-dashboards-plugin is in session management mode only. Session management mode is a mode that has log in and log out functionality as well as the ability to switch tenants, view user info and change the logged in user's password.
  2. Security Admin Pages - In the second instance, the security dashboards plugin operates as a shell and only adds the Security pages to OSD. This plugin does not provide the session management functionality.

cwperks added 28 commits May 17, 2024 13:28
…ng to be able to disable security admin pages

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
cwperks added 17 commits May 22, 2024 15:37
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
This reverts commit 018cc7f.
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Copy link
Collaborator

@RyanL1997 RyanL1997 left a comment

Choose a reason for hiding this comment

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

Hi @cwperks, thanks for putting this together. This is indeed a very interesting PR to review. I can see that this experimental change is designed to support MDS. To help bridge my knowledge gap with the new security frontend experience, I've left some conceptual questions in this round of review.


- uses: actions/checkout@v2
with:
path: ./OpenSearch-Dashboards/plugins/security-admin-dashboards-plugin
Copy link
Collaborator

@RyanL1997 RyanL1997 May 24, 2024

Choose a reason for hiding this comment

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

Is this the second installation for Security Admin Pages?


- uses: actions/checkout@v2
with:
path: ${{ steps.determine-plugin-directory.outputs.plugin-directory }}
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be the first installation for Session Management Only Mode, is that correct?

working-directory: OpenSearch-Dashboards
shell: bash

- name: Create opensearch_dashboards.json for security admin dashboards plugin
Copy link
Collaborator

Choose a reason for hiding this comment

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

I suggest we add a working-directory for this at ./OpenSearch-Dashboards/plugins/security-admin-dashboards-plugin

- name: Add to opensearch_dashboards.yml config file
run: |
echo 'opensearch_security.configuration.admin_pages_enabled: false' >> ./OpenSearch-Dashboards/config/opensearch_dashboards.yml
echo 'opensearch_security_admin.configuration.session_management_enabled: false' >> ./OpenSearch-Dashboards/config/opensearch_dashboards.yml
Copy link
Collaborator

Choose a reason for hiding this comment

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

For my knowledge, I do have some conceptual questions about these flags:

  • what are these flags for? (I guess it is for disable/enable the 2 security dashboards we installed, but why we disabled both of them here?)
  • Are they enabled by default?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Notice that the prefixes are different opensearch_security vs opensearch_security_admin.

For one of the plugin installations it disables the Admin pages and only manages the session, including login/logout, changing tenant and viewing user info. For the other plugin, it disables session management. In that mode, the plugin acts like any other dashboard plugin where it displays pages for the user to interact with security. It takes for granted that another plugin manages the session.

cwperks added 2 commits May 24, 2024 09:27
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
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.

2 participants