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

Separate HMAC and token authorization headers #37

Merged
merged 4 commits into from
May 13, 2020
Merged

Conversation

graft
Copy link
Contributor

@graft graft commented Apr 19, 2020

This is a PR meant to support the addition of two forms of authorization to a single request, that is: application-based authorization (hmac-signed) and user-based authentication (token). Hitherto, endpoints have made use of one or the other, but there has not been the requirement that we use BOTH forms of authorization at once; instead, we have used token-based authentication to generate HMAC urls, which are then submitted token-free to e.g. download a file.

However, a use case has appeared: allowing Magma to control what gets copied into its bucket on metis, while the user determines whether they have access to the file or the bucket in the first place. We therefore need to amend Etna::Auth to allow this possibility.

The main change in this PR happens here:

-     hmac_signature = auth(request, :hmac)
+     hmac_signature = etna_param(request, :signature)

Previously, both the token and the HMAC signature went into the "authorization" field (which could either be the header named "Authorization" or the param "X-Etna-Authorization" - no, it doesn't work to set one in the header and the other in the param). This means we can't pass in both at the same time. Here, we are instead moving the HMAC signature to the param or header "X-Etna-Signature", allowing us to send the token in the 'Authorization' header.

There are two other less significant changes which make up most of the noise:

  1. Previously request.env['etna.hmac'] would merely be set to true by Etna::Auth. Now it saves the HMAC object instead and makes it available later in the request (allowing the controller, for example, to easily check the id of the signing application and the signature validity).

  2. The TestAuth class is brought closer in line with the Etna::Auth class, and some more helpers are added to allow testing of HMAC features without having to correctly sign things.

Copy link
Collaborator

@coleshaw coleshaw left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@graft graft merged commit 7a527f7 into master May 13, 2020
@graft graft deleted the graft-repair-hmac branch June 6, 2020 01:36
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