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

auth: add www-authenticate based on user agent #1009

Merged
merged 27 commits into from
Dec 4, 2020
Merged

auth: add www-authenticate based on user agent #1009

merged 27 commits into from
Dec 4, 2020

Conversation

refs
Copy link
Member

@refs refs commented Dec 2, 2020

What?

We now comply with HTTP spec by adding Www-Authenticate headers on every 401 request. Furthermore, we not only take care of such a thing at the Proxy but also Reva will take care of it. In addition, we now are able to lock-in a set of User-Agent to specific challenges.

Admins can use this feature by configuring OCIS + Reva following this approach:

STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT="mirall:basic, Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0:bearer" \
PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT="mirall:basic, Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:83.0) Gecko/20100101 Firefox/83.0:bearer" \
PROXY_ENABLE_BASIC_AUTH=true \
go run cmd/ocis/main.go server

We introduced two new environment variables:

STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT as well as PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT, The reason they have the same value is not to rely on the os env on a distributed environment, so in redundancy we trust. They both configure the same on the backend storage and OCIS Proxy. Both variables are comma separated tuples of User-Agent:challenge.

#1009

TODO

  • documentation on how to run
  • check again code readability, this feature is confusing because of the underlying auth (backend / reva auth)
  • fix linter
  • make use of StatusReader to provide clarity on the logs

Technical Debt added while working on ocis-1132

  • Adding Www-Authenticate depends on the endpoint. Endpoints that delegate authorization to Reva will already contain Www-Authenticate headers that it was configured with. There should be 2 parameters to regulate a) the endpoints that are not delegating authentication to Reva and b) a set of user agents that should delegate authentication to Reva, depending on the endpoint. For instance:
    • an unauthorized request lands on .../ocs/...
      • proxy handles it
      • writes 401 header
      • writes Www-Authenticate headers as per configuration
    • an authorized request lands on .../ocs/...
      • proxy authorizes it
    • StringSlice flags on urfavecli are split on comma
      • STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT=mirall:basic,basic:bearer
    • document all endpoints that do not delegate authorization to reva gateway
  • Should the user-agent then for an old client behave the same way it does on reva when it reaches an endpoint such as /ocs/ that does not delegate requests to Reva?

Scenarios

  • unauthorized request, no user agent to WebDAV.
    • proxy adds www-authenticate headers.
    • reva adds www-authenticate headers.
    • curl -v -k -H "depth: 0" -X PROPFIND https://localhost:9200/remote.php/dav/files | xmllint --format -
  • unauthorized request, whitelisted user agent to WebDAV.
    • proxy removes www-authenticate headers on whitelisted user agent (from config).
    • reva adds www-authenticate headers for whitelisted user agent (from config).
    • curl -v -k -H "User-Agent: mirall" -H "depth: 0" -X PROPFIND https://localhost:9200/remote.php/dav/files | xmllint --format -
  • unauthorized request, no user agent to OCS.
    • proxy adds www-authenticate headers.
    • curl -k -v https://localhost:9200/ocs/v2.php/cloud/users/einstein | xmllint --format -
  • unauthorized request, whitelisted agent to OCS.
    • proxy adds www-authenticate headers for the whitelisted user agent and removes any other www-authenticate header present.
    • curl -k -v -H "User-Agent: mirall" https://localhost:9200/ocs/v2.php/cloud/users/einstein | xmllint --format -
  • authorized request to WebDAV.
    • no www-authenticate headers added.
  • authorized request to OCS.
    • no www-authenticate headers added.

Bottom line is:

  • When a user-agent whitelisted is configured, only one www-authenticate MUST be present, and it should be consistent across endpoints.

How to run this?

STORAGE_FRONTEND_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT=mirall:basic \
PROXY_MIDDLEWARE_AUTH_CREDENTIALS_BY_USER_AGENT=mirall:basic \
PROXY_ENABLE_BASIC_AUTH=true \
go run cmd/ocis/main.go server

Please do note that using environment variables is recommended as the runtime does not forward cli flags. This means when ocis server is ran, the server sub-command has no access to lower level flags, meaning even if ocis storage-frontend is ran under server, there is no way to inject a cli flag all the way down to the sub-sub command, this is a design choice of our runtime.

@update-docs
Copy link

update-docs bot commented Dec 2, 2020

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@refs refs requested a review from IljaN December 2, 2020 15:09
@refs refs self-assigned this Dec 2, 2020
@refs refs requested a review from butonic December 2, 2020 15:12
@refs refs marked this pull request as ready for review December 3, 2020 16:28
@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 4, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@labkode
Copy link
Contributor

labkode commented Dec 4, 2020

@refs looks good to me. General question: how can we configure OCIS with a config file with all the necessary values?

Copy link
Member

@IljaN IljaN left a comment

Choose a reason for hiding this comment

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

LGTM 👯

@IljaN IljaN self-requested a review December 4, 2020 15:28
@refs refs merged commit d446403 into master Dec 4, 2020
@refs refs deleted the ocis-1132 branch December 4, 2020 15:32
@refs refs mentioned this pull request Dec 4, 2020
@refs
Copy link
Member Author

refs commented Dec 4, 2020

@refs looks good to me. General question: how can we configure OCIS with a config file with all the necessary values?

Transcript from Gitter to have all within the same context:

I've been toying with a single point of entry for ocis and all its extensions, for that we use Viper. The proxy can be configured with a proxy.json that can be located in different locations, I always use /etc/ocis/proxy.jsonbut the problem is that the storage-* extensions (a.k.a Reva) have their configuration dynamically loaded, and not through Viper just yet
so we'd need to add support for config parsing with Viper for reva extensions
so there's a disconnect there

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.

3 participants