[Question] How to approach api/status api/reporting/stats route authentication addition in future releases

[peterzhuamazon]

With security team merge the change of adding auth to /api/status api/reporting/stats call in 2.0 branch, we are facing multiple issues with both build repo scripts/workflows, as well as the approach on future releases.

• Note: This change is not in 1.3 but only 2.0+, means we also need to think about how to maintain either 2 copies of change or choose a one-fit-all direction in build repo scripts.

---

Behavior changes

• Previously:

curl localhost:5601/api/status
curl localhost:5601/api/reporting/stats

• New:

curl localhost:5601/api/status -u admin:admin
curl localhost:5601/api/reporting/stats -u admin:admin

---

Two ways to approach this issue:

1. Restore previous behavior in 2.0 by adding this config into the opensearch_dashboards.yml:

 opensearch_security.auth.unauthenticated_routes: ['/api/reporting/stats', '/api/status']

2. Use new behavior by default in 2.0.

---

Impacts on build:

1. If we use previous behavior by edit opensearch_dashboards.yml:

• Nothing else changes
• Customer dont see the changes as well.

2. If we use the new behavior:

• All calls in build scripts that uses the old curl localhost:5601/api/status call needs to make changes
• This include Test framework, RPM validation workflow.
  • Additional impact on all the workflow runs on 1.3 branch, as 1.3 does not have this change so /api/status would run, but in 2.0 we need to run api/status -u admin:admin.
• Detailed documentation to inform user on this change as it is pretty big.

---

Additional items:

1. @cliu123 create documentation website related changes: [Default Behavior Change] Enforce authentication on api/status route documentation-website#563

Thanks.

[peterzhuamazon]

cc: @CEHENKLE @bbarani @dblock @cliu123

[dblock]

Is this another piece of code that needs to branch version-to-version, aka #1975?

For this particular problem, will specifying admin:admin be just ignored in the 1.3.x version? Will it hurt to add it without branching?

[peterzhuamazon]

Is this another piece of code that needs to branch version-to-version, aka #1975?

For this particular problem, will specifying admin:admin be just ignored in the 1.3.x version? Will it hurt to add it without branching?

1. Yes
2. I test on artifacts that do not have these change with these results:

% curl localhost:5601/api/reporting/stats
{"report":{"create":{"count":0,"system_error":0,"user_error":0,"total":0},"create_from_definition":{"count":0,"system_error":0,"user_error":0,"total":0},"download":{"count":0,"system_error":0,"user_error":0,"total":0},"list":{"count":0,"system_error":0,"user_error":0,"total":0},"info":{"count":0,"system_error":0,"user_error":0,"total":0}},"report_definition":{"create":{"count":0,"system_error":0,"user_error":0,"total":0},"list":{"count":0,"system_error":0,"user_error":0,"total":0},"info":{"count":0,"system_error":0,"user_error":0,"total":0},"update":{"count":0,"system_error":0,"user_error":0,"total":0},"delete":{"count":0,"system_error":0,"user_error":0,"total":0}},"report_source":{"list":{"count":0,"system_error":0,"user_error":0,"total":0}},"dashboard":{"pdf":{"download":{"count":0,"total":0}},"png":{"download":{"count":0,"total":0}}},"visualization":{"pdf":{"download":{"count":0,"total":0}},"png":{"download":{"count":0,"total":0}}},"notebook":{"pdf":{"download":{"count":0}},"png":{"download":{"count":0}}},"saved_search":{"csv":{"download":{"count":0,"total":0}}},"request_total":0,"request_count":0,"success_count":0,"failed_request_count_system_error":0,"failed_request_count_user_error":0}%

% curl localhost:5601/api/reporting/stats -u admin:admin
{"report":{"create":{"count":0,"system_error":0,"user_error":0,"total":0},"create_from_definition":{"count":0,"system_error":0,"user_error":0,"total":0},"download":{"count":0,"system_error":0,"user_error":0,"total":0},"list":{"count":0,"system_error":0,"user_error":0,"total":0},"info":{"count":0,"system_error":0,"user_error":0,"total":0}},"report_definition":{"create":{"count":0,"system_error":0,"user_error":0,"total":0},"list":{"count":0,"system_error":0,"user_error":0,"total":0},"info":{"count":0,"system_error":0,"user_error":0,"total":0},"update":{"count":0,"system_error":0,"user_error":0,"total":0},"delete":{"count":0,"system_error":0,"user_error":0,"total":0}},"report_source":{"list":{"count":0,"system_error":0,"user_error":0,"total":0}},"dashboard":{"pdf":{"download":{"count":0,"total":0}},"png":{"download":{"count":0,"total":0}}},"visualization":{"pdf":{"download":{"count":0,"total":0}},"png":{"download":{"count":0,"total":0}}},"notebook":{"pdf":{"download":{"count":0}},"png":{"download":{"count":0}}},"saved_search":{"csv":{"download":{"count":0,"total":0}}},"request_total":0,"request_count":0,"success_count":0,"failed_request_count_system_error":0,"failed_request_count_user_error":0}%

Seems like you are right when adding admin:admin on old cluster, it will just get ignored in 2.0.0-rc1 and 1.3.x.

So it seems like we can just add it by default, update documentation website, and announce this as part of the breaking changes.

Thanks.

[cliu123]

That's expected behavior. Without the change, credential is not required because it bypasses authentication by default. So no matter whether a credential is provided or not, request goes through, because the credential doesn't get evaluated at all.

[cliu123]

So it seems like we can just add it by default, update documentation website, and announce this as part of the breaking changes.

1. The issue to change the documentation: [Default Behavior Change] Enforce authentication on api/status route documentation-website#563
2. Added to breaking changes for 2.0.0 release: [Documentation] Breaking changes in 2.0 OpenSearch#2480 (comment)

[peterzhuamazon]

@cliu123 @dblock I just realize one thing does plugin owners requires to change their cypress test?
cc: @tianleh @kavilla

[cliu123]

I think they do. Whereever api/status is requested without credential in 2.0.0 needs to be changed and provide right credential(admin:admin).

[peterzhuamazon]

I think they do. Whereever api/status is requested without credential in 2.0.0 needs to be changed and provide right credential(admin:admin).

Thanks @cliu123.

@tianleh @kavilla in FT repo do you guys use api/status at all for cypress connection?
If not then you dont need to fix this as @zelinh is fixing the api/status on the build repo integTest framework.

Thanks.

[tianleh]

I think they do. Whereever api/status is requested without credential in 2.0.0 needs to be changed and provide right credential(admin:admin).

Thanks @cliu123.

@tianleh @kavilla in FT repo do you guys use api/status at all for cypress connection? If not then you dont need to fix this as @zelinh is fixing the api/status on the build repo integTest framework.

Thanks.

Thanks for the reminder. The Cypress tests just simulate customers' behavior to click around the UI and thus doesn't need to run such api/status. We do have Github actions which need to check the api/status to know OSD health. See opensearch-project/opensearch-dashboards-functional-test#230 (comment)