-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add support for shared token for authentication #18
Open
sakisv
wants to merge
5
commits into
Crown-Commercial-Service:master
Choose a base branch
from
alphagov:master
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is a backwards compatible change which will allow for a pre-shared token to be distributed with our apps and get prometheus to use this token to authenticate itself. This results in a much simpler deployment model where prometheus scraping requests don't need to be proxied through another server and can allow for hosting prometheus on PaaS.
sakisv
added a commit
to alphagov/notifications-api
that referenced
this pull request
Apr 12, 2022
These routes will be used by prometheus to scrape the `/metrics` endpoint. Currently: The shared prometheus scrapes the `/metrics` endpoint using the public routes. The `/metrics` endpoint is provided by the [gds_metrics_python][] which comes with [bearer-token authentication][] where the token is expected to be equal to the paas app id. Each app is configured as a separate target in the shared prometheus with its app id configured as a GET parameter (e.g. http://notify-api-production.cloudapps.digital/metrics?cf_app_guid=69c87503-6b53-4c35-XXXX-XXXXXXXXXXXX&cf_app_instance=69c87503-6b53-4c35-XXXX-XXXXXXXXXXXX%3A1&cf_app_instance_index=1) Each scrape request goes through an nginx proxy which retrieves this argument from the query string and sets it as a header [source][]. This way it passes the authentication and also is able to instruct the gorouter to target a specific instance of the app. In the future: Since we're moving away from the shared prometheus and towards an approach where we [run our own prometheus on PaaS][] we can skip the need for having an nginx proxy and use the internal routes instead, and have a [preshared-token][] for authentication if we need to. [gds_metrics_python]: https://github.com/Crown-Commercial-Service/gds_metrics_python [bearer-token authentication]: https://github.com/Crown-Commercial-Service/gds_metrics_python/blob/master/gds_metrics/__init__.py#L47-L52 [source]: https://github.com/alphagov/prometheus-aws-configuration-beta/blob/master/terraform/modules/prom-ec2/prometheus/cloud.conf#L111-L123 [run our own prometheus on PaaS]: alphagov/notifications-cf-monitoring#1 [preshared-token]: Crown-Commercial-Service/gds_metrics_python#18
sakisv
added a commit
to alphagov/notifications-api
that referenced
this pull request
Apr 13, 2022
These routes will be used by prometheus to scrape the `/metrics` endpoint. Currently: The shared prometheus scrapes the `/metrics` endpoint using the public routes. The `/metrics` endpoint is provided by the [gds_metrics_python][] which comes with [bearer-token authentication][] where the token is expected to be equal to the paas app id. Each app is configured as a separate target in the shared prometheus with its app id configured as a GET parameter (e.g. http://notify-api-production.cloudapps.digital/metrics?cf_app_guid=69c87503-6b53-4c35-XXXX-XXXXXXXXXXXX&cf_app_instance=69c87503-6b53-4c35-XXXX-XXXXXXXXXXXX%3A1&cf_app_instance_index=1) Each scrape request goes through an nginx proxy which retrieves this argument from the query string and sets it as a header [[source][]]. This way it passes the authentication and also is able to instruct the gorouter to target a specific instance of the app. In the future: Since we're moving away from the shared prometheus and towards an approach where we [run our own prometheus on PaaS][] we can skip the need for having an nginx proxy and use the internal routes instead, and have a [preshared-token][] for authentication if we need to. [gds_metrics_python]: https://github.com/Crown-Commercial-Service/gds_metrics_python [bearer-token authentication]: https://github.com/Crown-Commercial-Service/gds_metrics_python/blob/master/gds_metrics/__init__.py#L47-L52 [source]: https://github.com/alphagov/prometheus-aws-configuration-beta/blob/master/terraform/modules/prom-ec2/prometheus/cloud.conf#L111-L123 [run our own prometheus on PaaS]: alphagov/notifications-cf-monitoring#1 [preshared-token]: Crown-Commercial-Service/gds_metrics_python#18
Separate the two possible authentication tokens to more descriptive variables and introduce a dedicated variable to flag whether we need to authencate the requests or not
First change the name of the variable to be more reflective of its content Second and more important, test that both the app_id and the preshared token can co-exist and the request will be allowed
This maintains the current behaviour of the system and it adds support for both authentication methods So if an app: - has `METRICS_BASIC_AUTH` enabled (which defaults to true) - AND is running on PaaS (i.e. it has an application_id) - AND receives a request with the auth_header properly configured - AND has either an auth_token OR an application_id that matches the one in the auth_header Then the request will be allowed
Flexible authentication
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a backwards compatible change which will allow for a pre-shared
token to be distributed with our apps and get prometheus to use this
token to authenticate itself.
This results in a much simpler deployment model where prometheus
scraping requests don't need to be proxied through another server and
can allow for hosting prometheus on PaaS.