Added support for authentication processor #1728

jpkrohling · 2020-09-03T09:51:59Z

Description: Added a new processor, to take care of authentication for incoming requests

Link to tracking Issue: #1424

Testing: unit tests + manual tests

Documentation: readme included

Includes an initial support for OIDC authenticator.

Closes #1424

Signed-off-by: Juraci Paixão Kröhling [email protected]

codecov · 2020-09-03T10:01:57Z

Codecov Report

Merging #1728 into master will increase coverage by 0.02%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##           master    #1728      +/-   ##
==========================================
+ Coverage   91.90%   91.92%   +0.02%     
==========================================
  Files         262      265       +3     
  Lines       18744    18894     +150     
==========================================
+ Hits        17226    17369     +143     
- Misses       1085     1090       +5     
- Partials      433      435       +2

Impacted Files	Coverage Δ
config/configgrpc/configgrpc.go	`96.62% <57.14%> (-3.38%)`	⬇️
internal/auth/oidc_authenticator.go	`98.24% <98.24%> (ø)`
internal/auth/authenticator.go	`100.00% <100.00%> (ø)`
internal/auth/context.go	`100.00% <100.00%> (ø)`
translator/internaldata/resource_to_oc.go	`87.87% <0.00%> (-2.03%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 471c4a6...b1a47a6. Read the comment docs.

jpkrohling · 2020-09-09T12:33:13Z

@bogdandrutu could you please review this one, or assign someone to do it?

bogdandrutu

Please try to follow the rules from contributing guidelines:
https://github.com/open-telemetry/opentelemetry-collector/blob/master/CONTRIBUTING.md#when-adding-a-new-component

bogdandrutu · 2020-09-15T21:10:40Z

processor/authenticationprocessor/README.md

+* Supported pipeline types: traces
+* Status: in development
+
+This processor authenticates the incoming traces by extracting the authentication data from the context


s/traces/telemetry

bogdandrutu · 2020-09-15T21:11:58Z

processor/authenticationprocessor/factory.go

+	oCfg := cfg.(*Config)
+	if oCfg.OIDC == nil {
+		return nil, errNoOIDCProvided
+	}
+	if oCfg.OIDC.Audience == "" {
+		return nil, errNoClientIDProvided
+	}
+	if oCfg.OIDC.IssuerURL == "" {
+		return nil, errNoIssuerURL
+	}


extract this in a separate func "validateConfig". We should be able to reuse everything for metrics/logs as well.

bogdandrutu · 2020-09-15T21:14:01Z

processor/authenticationprocessor/README.md

+and verifying the bearer token with the specified provider.
+
+Currently, only bearer token authentication is supported, added as part of `PerRPC` gRPC authentication.
+It requires the gRPC client to send a header named `authorization` in line with the equivalent HTTP/2 header.


Can we have the header name as a config option with a default value "authorization"?

bogdandrutu · 2020-09-15T21:14:49Z

processor/authenticationprocessor/config.go

+}
+
+// OIDC defines the OpenID Connect properties for this processor
+type OIDC struct {


ss/OIDC/OIDCSettings

bogdandrutu · 2020-09-15T21:15:23Z

processor/authenticationprocessor/doc.go

+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package authenticationprocessor


No need to have an empty doc.go

tigrannajaryan · 2020-09-09T23:12:20Z

processor/authenticationprocessor/README.md

@@ -0,0 +1,23 @@
+# Authentication Processor
+
+* Supported pipeline types: traces


Any reason only traces can be supported?

jpkrohling · 2020-09-16T08:22:56Z

I'm converting this PR to draft, as @pavolloffay had very good suggestions on how to make this better. Once discussion has been concluded as part of #1424, I'll update this PR with the new proposal.

jpkrohling · 2020-09-17T13:17:08Z

This PR is ready for review again, moving the auth logic from a processor to interceptors. In this first iteration, it's only for gRPC receivers, but I'll work on HTTP receivers in a follow-up PR, as this PR is already big enough.

jpkrohling · 2020-09-17T13:40:02Z

This has been tested manually with Auth0 as the OIDC provider. The following configuration was used during the manual tests:

receivers:
  otlp/noauth:
    protocols:
      grpc:
        endpoint: localhost:55680
  otlp/auth:
    protocols:
      grpc:
        endpoint: localhost:56680
        tls_settings:
          cert_file: ./local/self-signed.pem
          key_file: ./local/self-signed-key.pem
        auth:
          oidc:
            issuer_url: https://dev-zkkhf874.eu.auth0.com/
            audience: http://auth.example.com

processors:
  batch:

exporters:
  logging:
  otlp/auth:
    endpoint: localhost:56680
    ca_file: ./local/ca.pem
    per_rpc_auth:
      type: bearer
      bearer_token: eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImxiWmJrd2JHUXlrWGdub09mRXNRNCJ9.eyJpc3MiOiJodHRwczovL2Rldi16a2toZjg3NC5ldS5hdXRoMC5jb20vIiwic3ViIjoiMUpEUmZJMm43c0JhNnhvMkQ2VEJSVnpjbGs5M25uVUZAY2xpZW50cyIsImF1ZCI6Imh0dHA6Ly9hdXRoLmV4YW1wbGUuY29tIiwiaWF0IjoxNjAwMzM4MTU4LCJleHAiOjE2MDA0MjQ1NTgsImF6cCI6IjFKRFJmSTJuN3NCYTZ4bzJENlRCUlZ6Y2xrOTNublVGIiwiZ3R5IjoiY2xpZW50LWNyZWRlbnRpYWxzIn0.qU8CSyIgD6KML5Z1hvjKITLlysX6GNPAECsUSp7FxyGW4ryygICcuwqJ85nAv8Xvgrg9EMITG0GY3MlvzokzfWgrAT_CncQ9_Aaxj3-BRvcOvHUur2fUNLE53Pqj1kroVt21333Ly5SqwLQb-DEiEKDXnw1u70clFaxQs4eT9xmvjqY9FX3jW-fh2lbUkURv3opuA0F1nerUbGvX9wD-pToMZUGHwNScf1VrN-Um8i36WBwgWmeGmVIcgzZHMEVxs8AbvLOnyOlCojnshJod3VFqOriH47F4NvhfK0YABSOt_4Q1AVHHFDQ2J_9oI0gRvj-FvU5TxLBsQ-Fqd6AhxA
  jaeger:
    endpoint: localhost:14250
    insecure: true

service:
  pipelines:
    traces:
      receivers:
        - otlp/noauth
      processors:
        - batch
      exporters:
        - otlp/auth
    traces/2:
      receivers:
        - otlp/auth
      processors:
        - batch
      exporters:
        - jaeger

Includes an initial support for OIDC authenticator. Closes open-telemetry#1424 Signed-off-by: Juraci Paixão Kröhling <[email protected]>

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

bogdandrutu

Consider to split this in smaller and more readable PRs. Adding the configauth; add implementation; hook into configgrpc;

bogdandrutu · 2020-09-17T15:01:17Z

internal/auth/README.md

@@ -0,0 +1,17 @@
+# Authentication mechanism for servers


This probably can go to the configauth. That is the main package, here is just the implementation.

jpkrohling · 2020-09-18T08:52:47Z

Consider to split this in smaller and more readable PRs. Adding the configauth; add implementation; hook into configgrpc;

Done:

jpkrohling · 2020-09-18T09:03:13Z

Closing this in favor of the 4 PRs listed above.

Use the newly added SpanContext.IsRemote method instead.

jpkrohling requested review from bogdandrutu, dmitryax, james-bebbington, nilebox, owais, pjanotti and tigrannajaryan as code owners September 3, 2020 09:52

jpkrohling force-pushed the jpkrohling/authentication-processor branch from 975159a to d2ab657 Compare September 3, 2020 10:01

This comment has been minimized.

Sign in to view

jpkrohling force-pushed the jpkrohling/authentication-processor branch from 1330125 to dddab34 Compare September 7, 2020 14:02

tigrannajaryan self-assigned this Sep 9, 2020

bogdandrutu reviewed Sep 15, 2020

View reviewed changes

tigrannajaryan reviewed Sep 16, 2020

View reviewed changes

jpkrohling marked this pull request as draft September 16, 2020 08:22

jpkrohling force-pushed the jpkrohling/authentication-processor branch from dddab34 to e6adf0a Compare September 17, 2020 13:13

jpkrohling marked this pull request as ready for review September 17, 2020 13:16

Added auth settings for receivers.

8c61692

Includes an initial support for OIDC authenticator. Closes open-telemetry#1424 Signed-off-by: Juraci Paixão Kröhling <[email protected]>

jpkrohling force-pushed the jpkrohling/authentication-processor branch from e6adf0a to 8c61692 Compare September 17, 2020 13:47

jpkrohling added 2 commits September 17, 2020 16:05

lint and fixed defaultcomponents test

7b5cbf5

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

added empty_test to configauth

b1a47a6

Signed-off-by: Juraci Paixão Kröhling <[email protected]>

bogdandrutu reviewed Sep 17, 2020

View reviewed changes

flands assigned bogdandrutu Sep 17, 2020

jpkrohling mentioned this pull request Sep 18, 2020

Authentication Helper 1/4 - Add configauth #1807

Merged

This was referenced Sep 18, 2020

Authentication processor 2/4 - Add auth context and interface #1808

Merged

Authentication processor 3/4 - Add implementation #1809

Merged

Authentication processor 4/4 - Add configauth to configgrpc #1810

Merged

jpkrohling closed this Sep 18, 2020

MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021

Remove hasRemoteParent field from SDK span (open-telemetry#1728)

382c7ce

Use the newly added SpanContext.IsRemote method instead.

hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023

Only use config server if env var unset (open-telemetry#1728)

0adc731

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added support for authentication processor #1728

Added support for authentication processor #1728

jpkrohling commented Sep 3, 2020

codecov bot commented Sep 3, 2020 •

edited

Loading

This comment has been minimized.

jpkrohling commented Sep 9, 2020

bogdandrutu left a comment

bogdandrutu Sep 15, 2020

bogdandrutu Sep 15, 2020

bogdandrutu Sep 15, 2020

bogdandrutu Sep 15, 2020

bogdandrutu Sep 15, 2020

tigrannajaryan Sep 9, 2020

jpkrohling commented Sep 16, 2020

jpkrohling commented Sep 17, 2020

jpkrohling commented Sep 17, 2020

bogdandrutu left a comment

bogdandrutu Sep 17, 2020

jpkrohling commented Sep 18, 2020

jpkrohling commented Sep 18, 2020

		@@ -0,0 +1,23 @@
		# Authentication Processor

		* Supported pipeline types: traces

Added support for authentication processor #1728

Added support for authentication processor #1728

Conversation

jpkrohling commented Sep 3, 2020

codecov bot commented Sep 3, 2020 • edited Loading

Codecov Report

This comment has been minimized.

jpkrohling commented Sep 9, 2020

bogdandrutu left a comment

Choose a reason for hiding this comment

bogdandrutu Sep 15, 2020

Choose a reason for hiding this comment

bogdandrutu Sep 15, 2020

Choose a reason for hiding this comment

bogdandrutu Sep 15, 2020

Choose a reason for hiding this comment

bogdandrutu Sep 15, 2020

Choose a reason for hiding this comment

bogdandrutu Sep 15, 2020

Choose a reason for hiding this comment

tigrannajaryan Sep 9, 2020

Choose a reason for hiding this comment

jpkrohling commented Sep 16, 2020

jpkrohling commented Sep 17, 2020

jpkrohling commented Sep 17, 2020

bogdandrutu left a comment

Choose a reason for hiding this comment

bogdandrutu Sep 17, 2020

Choose a reason for hiding this comment

jpkrohling commented Sep 18, 2020

jpkrohling commented Sep 18, 2020

codecov bot commented Sep 3, 2020 •

edited

Loading