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

ASAP Client Authentication Extension #6627

Merged

Conversation

jamesmoessis
Copy link
Contributor

@jamesmoessis jamesmoessis commented Dec 8, 2021

I have raised this PR as my branch was messed up in #6233 and needed to start on a fresh one

Description:
Adding an extension which allows the collector to authenticate outgoing requests with ASAP (Atlassian Service Authentication Protocol).

It works by implementing ClientAuthenticator and setting the authorization header as the encoded JWT. It uses the go-asap library to achieve much of its functionality.

Link to tracking Issue:
#5930

Testing:
We mock key server and a base RoundTripper to achieve validation on signature and fields on the JWT. Configuration and factory are also tested. There are yaml files for good and bad config (one good, multiple bad configurations).

Additionally, at Atlassian we have been using ASAP in the collector for a long time already.

Documentation:

README.md gives a sample configuration and brief description. Those familiar with ASAP should have little trouble setting it up

Additionally, @MovieStoreGuy and myself are happy to be the codeowners for this extension.

@jamesmoessis
Copy link
Contributor Author

jamesmoessis commented Dec 8, 2021

@jpkrohling I have addressed your comments from the old PR:

You also asked

For my own education, do you have a resource with the differences between it and OAuth 2?

I don't know of a page that exactly outlines what you're asking, but I can explain some key differences here. Some differences:

  • Asap is purely for service-to-service.
  • In Asap the client self issues its tokens. The server verifies the token using a public key obtained from a keyserver.
  • Asap does not concern itself with authorisation. Its "primary goal is to allow the resource server to be confident that an incoming request comes from an authentic, identifiable client." (from the spec)

Sorry about the old PR getting corrupted! Let me know if there's anything else I can do to improve the PR 🙂

@jamesmoessis jamesmoessis changed the title asapauthextension + added to versions.yaml Asap Client Authentication Extension Dec 8, 2021
@jpkrohling jpkrohling requested review from jpkrohling and removed request for owais December 8, 2021 08:41
@jpkrohling jpkrohling assigned jpkrohling and unassigned kbrockhoff Dec 8, 2021
@jpkrohling jpkrohling changed the title Asap Client Authentication Extension ASAP Client Authentication Extension Dec 8, 2021
Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

LGTM. A small comment about the name of the comment and the data type for TTL. Nothing serious, and can be merged as it is if you don't think those changes are desired.

extension/asapauthextension/extension.go Outdated Show resolved Hide resolved
extension/asapauthextension/config.go Outdated Show resolved Hide resolved
@jpkrohling jpkrohling merged commit 5da06f3 into open-telemetry:main Dec 9, 2021
PaurushGarg referenced this pull request in open-o11y/opentelemetry-collector-contrib Dec 11, 2021
* asapauthextension + added to versions.yaml

* address jpkrohling comments

* reorder in correct alphabetical

* Asap -> ASAP name change

* update cfg to receive time.Duration directly, and reflect in tests and docs

* appease linter
PaurushGarg referenced this pull request in open-o11y/opentelemetry-collector-contrib Dec 11, 2021
* asapauthextension + added to versions.yaml

* address jpkrohling comments

* reorder in correct alphabetical

* Asap -> ASAP name change

* update cfg to receive time.Duration directly, and reflect in tests and docs

* appease linter
PaurushGarg referenced this pull request in open-o11y/opentelemetry-collector-contrib Dec 14, 2021
* asapauthextension + added to versions.yaml

* address jpkrohling comments

* reorder in correct alphabetical

* Asap -> ASAP name change

* update cfg to receive time.Duration directly, and reflect in tests and docs

* appease linter
povilasv referenced this pull request in coralogix/opentelemetry-collector-contrib Dec 19, 2022
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>

Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
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.

4 participants