WIP Credential factory - #547
Conversation
…e for AWS GetFederationToken
ghost
assigned
|
|
||
| # TODO | ||
|
|
||
| * Should the `expiration` result be set as a header instead? |
There was a problem hiding this comment.
Seems to me that the standard Expires header fits the bill perfectly.
| gem 'pg' | ||
| gem 'sequel-postgres-schemata', require: false | ||
|
|
||
| gem 'aws-sdk-core' |
There was a problem hiding this comment.
Not a fan of adding this to the Gemfile at the base level. Can we try to load the gems dynamically and disable the feature if they're not available? We can keep them in Gemfile in a separate group so that it's easier to test and deploy, while still possible to disable.
| logger.debug "#{e}\n#{e.backtrace.join "\n"}" | ||
| head :forbidden | ||
| if e.message != "ApplicationController::Forbidden" | ||
| render json: { |
There was a problem hiding this comment.
I think this should render text unless JSON is explicitly requested in an Accept header.
| def values factory_resource | ||
| provider_annotation = factory_resource.credential_factory_provider | ||
| raise ArgumentError, %Q(Annotation "credential-factory/provider" not found on #{factory_resource.id.inspect}) unless provider_annotation | ||
| require "credential_factory/#{provider_annotation.value.underscore}" |
There was a problem hiding this comment.
This will fail with 500 if the plugin is not found. I'm not sure what the correct code would be -- it's not exactly a client error, so suggests 5xx. Maybe 404, with an informative message? An informative message would be nice regardless of the code.
| require "credential_factory/#{provider_annotation.value.underscore}" | ||
| factory_class = self.const_get(provider_annotation.value.downcase.camelize) | ||
|
|
||
| annotations = factory_resource.annotations.select do |annotation| |
There was a problem hiding this comment.
It's not clear to me how that'll work with the database, or if at all. What do you think about using factory_resource.annotations.where(name: /^credential-factory/) to make the database do the filtering? Nb, I think it should be pretty harmless to pass provider annotation to the provider as well, and -- who knows -- maybe even useful.
| unless factory_resource.role.allowed_to?('execute', variable) | ||
| msg = "#{factory_resource.resource_id.inspect} does not have 'execute' privilege on #{variable.resource_id.inspect}" | ||
| Rails.logger.info msg | ||
| raise Exceptions::Forbidden, "#{factory_resource.resource_id.inspect} does not have 'execute' privilege on #{variable.resource_id.inspect}" |
There was a problem hiding this comment.
I wonder -- should these messages be exposed to the client?
| end | ||
| end | ||
| end | ||
| dependent_secrets = dependent_variables.inject({}) do |memo, variable| |
There was a problem hiding this comment.
Protip: instead of
xh = xs.inject({}) do |memo, x|
memo[k] = f(x)
memo
endyou can do
xh = xs.map do |x|
[k, f(x)]
end.to_h
| end | ||
|
|
||
| def annotations | ||
| resource.annotations.inject({}) do |memo, entry| |
There was a problem hiding this comment.
There's gotta be a better way. :/
| # From an example annotation name, build a set of annotation names which are related by | ||
| # the prefix on the example. If the example is 'aws/ci/secret_access_key' and the +names+ are | ||
| # 'foo' and 'bar', this method returns 'aws/ci/foo' and 'aws/ci/bar'. | ||
| def build_variable_ids example, names |
There was a problem hiding this comment.
Nitpick: this method produces a lot of garbage. How about names.map &example[/.*\//].method(:+)?
| "Effect" : "Allow", | ||
| "Resource" : "*" | ||
| } | ||
| ] |
There was a problem hiding this comment.
Gotta love YAML.
However, in the JSON heredoc you missed the closing brace.
|
Closing this as it's been stale for quite some time. |
3 participants
Closes [relevant GitHub issues, eg #76]
What does this pull request do?
A Credential Factory is a Conjur service which creates a new set of credentials each time it is called. A credential factory is a secure and scalable way to expose a credential-issuing backend, such as AWS Security Token Service, to Conjur clients.
See dependency PR - cyberark/conjur-policy-parser#9
What background context can you provide?
AWS Security Token Service (STS) is an AWS service which issues temporary credentials with a client-specified IAM policy.
Conjur Credential Factory is a way to expose the capabilities of STS-like services without over-privileging clients or requiring clients to directly handle permanent AWS credentials. A client can use the familiar "Conjur secrets fetch" interface to obtain a temporary set of AWS credentials with a set of privileges that are pre-defined by the security administrator. Many different credential factories can be defined on a single set of permanent AWS credentials. Access to credential factories is managed exactly the same way as access to variables, providing a familiar workflow and granular control.
Where should the reviewer start?
Read and understand the new cucumber tests and the CredentialFactory.md.
How should this be manually tested?
CredentialFactory.md shows how to try this from the Rails console. You can also invoke
GET /secrets/cucumber/webservice/myapp/aws-credentialsvia cURL (with a valid access token, of course).Screenshots (if appropriate)
Link to build in Jenkins (if appropriate)
https://jenkins.conjur.net/job/cyberark--conjur/job/feature%252Fcredential-factory/
Questions:
Yes
Yes
Not completely.
Yes