opensearch-project / security-dashboards-plugin

🔐 Manage your internal users, roles, access control, and audit logs from OpenSearch Dashboards
https://opensearch.org/docs/latest/security-plugin/index/
Apache License 2.0
71 stars 162 forks source link

[BUG] Invalid next URL when session expired #2135

Open Hailong-am opened 1 month ago

Hailong-am commented 1 month ago

What is the bug?

Here is the url with error

http://localhost:5601/app/login?nextUrl=%2Fapp%2Fopensearch_index_management_dashboards_%252Frollups#/rollups?from=0&search=&size=20&sortDirection=desc&sortField=_id

{
    "statusCode": 400,
    "error": "Bad Request",
    "message": "[request query.nextUrl]: Invalid nextUrl parameter."
}

The nextUrl is /app/opensearch_index_management_dashboards_%2Frollups Based on the validation rule, it has %2F which is not allowed.

https://github.com/opensearch-project/security-dashboards-plugin/blob/506d803b868002f131a65d0d7ad370625454a8e4/server/utils/next_url.ts#L75-L80

How can one reproduce the bug? Steps to reproduce the behavior:

  1. Enable workspace and goes into a workspace
  2. Go to http://localhost:5601/app/opensearch_index_management_dashboards_%2Frollups#/rollups?dataSourceId=&from=0&search=&size=20&sortDirection=desc&sortField=_id
  3. Wait for session expired
  4. See error

What is the expected behavior? A clear and concise description of what you expected to happen.

What is your host/environment?

Do you have any screenshots? If applicable, add screenshots to help explain your problem.

Do you have any additional context?

cwperks commented 1 month ago

@Hailong-am Why is the URL

localhost:5601/app/login?nextUrl=%2Fapp%2Fopensearch_index_managementdashboards%252Frollups#/rollups?from=0&search=&size=20&sortDirection=desc&sortField=_id

instead of

localhost:5601/app/login?nextUrl=%2Fapp%2Fopensearch_index_managementdashboards%2Frollups#/rollups?from=0&search=&size=20&sortDirection=desc&sortField=_id

i.e. Why the %25? (%25 is URL-encoded percent symbol) IMO the logic for nextUrl validation is sound, but the url you have pasted is doubly-encoded.

cwperks commented 1 month ago

@Hailong-am Is encodeUriComponent required when declaring the paths in the index-management-dashboards-plugin here?

Hailong-am commented 1 month ago

@Hailong-am Is encodeUriComponent required when declaring the paths in the index-management-dashboards-plugin here?

I think so, the application id opensearch_index_management_dashboards_${encodeURIComponent('/routes')}, if remove the encodeURIComponent the application id will be opensearch_index_management_dashboards_/routes, so url will be /app/opensearch_index_management_dashboards_/routes/... that will render application with id opensearch_index_management_dashboards_ which is not exists

cwperks commented 1 month ago

We would need to confirm if the route id is the cause of the double encoding. In the security-dashboards-plugin, route ids are explicitly defined and not based on the route configured. Examples: https://github.com/opensearch-project/security-dashboards-plugin/blob/main/public/plugin.ts#L189-L275

The ids are defined with underscores like this: https://github.com/opensearch-project/security-dashboards-plugin/blob/main/common/index.ts#L18-L24

Hailong-am commented 1 month ago

route ids are explicitly defined and not based on the route configured. Examples: https://github.com/opensearch-project/security-dashboards-plugin/blob/main/public/plugin.ts#L189-L275

yes, that's the cause. application id could not contains any special character like / +, otherwise it will have same problem.

either we have a place to document this limitation, or we figure out a way to check the application id is a valid and exist one no matter what kind of format does it have.