Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

Remove resetting Auth credentials in reverse-proxy #1349

Merged

Conversation

akatashev
Copy link
Contributor

@akatashev akatashev commented May 1, 2021

Related issue: #979

Description

This PR removes setting Auth credentials in reverse-proxy.
@andrein was right about the fact that lines

switch strings.ToLower(config.Config.AuthenticationMethod) {
case "basic", "multi":
r.SetBasicAuth(config.Config.HTTPAuthUser, config.Config.HTTPAuthPassword)
}

reset auth credentials for every request going through reverse-proxy and therefore provide this request read/write permissions even for a readonly user.

These lines are unnecessary, because there are 3 basic scenarios:

  1. Username and password are incorrect, so an attempt to sent a request receives 401 Unauthorized. In this case we shouldn't care about proxying, because it never reaches the proxy.
  2. Leader and follower have different configuration. For example, a follower has "AuthenticationMethod": "multi" and a leader has "AuthenticationMethod": "basic". But that isn't a proxy problem.
  3. Leader and follower are configured correctly, so if Authorization in a request is good for a follower, it should be good for a leader when request is proxied to it. So it means there is no need to modify authorisation for a request.

Testing

During the whole testing both servers had the following configuration:

{
  "AuthenticationMethod": "multi",
  "HTTPAuthUser":         "dba_team",
  "HTTPAuthPassword":     "time_for_dinner"
}

Before fix behaviour (Orchestrator version built from the latest master state):

bash-4.2$ curl https://orch-leader:3000/api/forget-cluster/mysql-cluster -k -u readonly:123
{"Code":"ERROR","Message":"Unauthorized","Details":null}
bash-4.2$ curl https://orch-follower:3000/api/forget-cluster/mysql-cluster -k -u readonly:123
{"Code":"OK","Message":"Cluster forgotten: mysql-server-1:3306","Details":null}

Request to the leader is rejected, because it's executed as readonly, but the same request sent to the follower is accepted, because its user is implicitly changed to dba_team by reverse-proxy.

After fix behaviour:

bash-4.2$ curl -k https://orch-leader:3000/api/forget-cluster/mysql-cluster -u readonly:123
{"Code":"ERROR","Message":"Unauthorized","Details":null}
bash-4.2$ curl -k https://orch-follower:3000/api/forget-cluster/mysql-cluster -u readonly:123
{"Code":"ERROR","Message":"Unauthorized","Details":null}

Since credentials are not rewritten by reverse-proxy, both requests produce expected results.

To ensure that we didn't break actual functionality, let's execute a valid request through a follower:

bash-4.2$ curl -k https://orch-follower:3000/api/forget-cluster/mysql-cluster -u dba_team:time_for_dinner
{"Code":"OK","Message":"Cluster forgotten: mysql-server-1:3306","Details":null}

It shows that the request was relayed and executed successfully.

Copy link
Collaborator

@shlomi-noach shlomi-noach left a comment

Choose a reason for hiding this comment

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

Thank you for the fix and for the analysis!

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.

2 participants