Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/libraries/go/lib/pkg/auth/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,10 @@ load("@rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "auth",
srcs = ["auth_token_fetcher.go"],
srcs = [
"auth_token_fetcher.go",
"authorizer.go",
],
importpath = "github.com/NVIDIA/nvcf/src/libraries/go/lib/pkg/auth",
visibility = ["//visibility:public"],
deps = [
Expand All @@ -29,7 +32,10 @@ go_library(

go_test(
name = "auth_test",
srcs = ["auth_token_fetcher_test.go"],
srcs = [
"auth_token_fetcher_test.go",
"authorizer_test.go",
],
data = glob(["test/**"]),
embed = [":auth"],
deps = [
Expand Down
200 changes: 200 additions & 0 deletions src/libraries/go/lib/pkg/auth/authorizer.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
/*
SPDX-FileCopyrightText: Copyright (c) NVIDIA CORPORATION & AFFILIATES. All rights reserved.
SPDX-License-Identifier: Apache-2.0

Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
You may obtain a copy of the License at

http://www.apache.org/licenses/LICENSE-2.0

Unless required by applicable law or agreed to in writing, software
distributed under the License is distributed on an "AS IS" BASIS,
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
See the License for the specific language governing permissions and
limitations under the License.
*/

package auth

import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io"
"net/http"
"time"
)

// Common authorization errors.
var (
ErrUnauthorized = errors.New("unauthorized: missing or invalid authentication credentials")
ErrForbidden = errors.New("forbidden: principal lacks permission for the requested action")
ErrAuthorizerInternal = errors.New("internal authorizer error")
)

// Action represents the permission or operation being requested.
// Consuming services should define their own Action constants in their own packages.
type Action string

// AuthRequest encapsulates the identity, action, and target resource for an authorization check.
type AuthRequest struct {
// Credential holds the token or secret presented by the client.
Credential string `json:"credential,omitempty"`
// OrgID identifies the organization or tenant context.
OrgID string `json:"org_id,omitempty"`
// PrincipalID identifies the user or service principal requesting access.
PrincipalID string `json:"principal_id,omitempty"`
// ResourceID identifies the target resource being accessed.
ResourceID string `json:"resource_id,omitempty"`
// Action specifies the operation requested on the resource.
Action Action `json:"action"`
}

// AuthResult represents the outcome of an authorization check.
type AuthResult struct {
// Allowed indicates whether the requested action is permitted.
Allowed bool `json:"allowed"`
// PrincipalID is the verified identity of the requester.
PrincipalID string `json:"principal_id,omitempty"`
// Scopes lists the granted permission scopes or roles.
Scopes []string `json:"scopes,omitempty"`
// Reason provides human-readable context when access is denied.
Reason string `json:"reason,omitempty"`
}

// Authorizer defines the public interface for evaluating access requests in open-source builds.
type Authorizer interface {
// Authorize evaluates whether the request is allowed to perform the action.
Authorize(ctx context.Context, req *AuthRequest) (*AuthResult, error)
}

// NoopAuthorizer implements Authorizer by allowing all requests.
// This implementation is intended for local development and standalone testing.
type NoopAuthorizer struct{}

// NewNoopAuthorizer creates a new NoopAuthorizer that allows every request.
func NewNoopAuthorizer() *NoopAuthorizer {
return &NoopAuthorizer{}
}

// Authorize always returns an allowed result.
func (a *NoopAuthorizer) Authorize(ctx context.Context, req *AuthRequest) (*AuthResult, error) {
if req == nil {
return nil, ErrUnauthorized
}
principal := req.PrincipalID
if principal == "" {
principal = "anonymous"
}
return &AuthResult{
Allowed: true,
PrincipalID: principal,
Scopes: []string{"*"},
}, nil
}

// noRedirectPolicy is an http.Client.CheckRedirect function that refuses all redirects.
// This prevents 307/308 responses from replaying a POST carrying AuthRequest.Credential
// to an unintended redirect target.
func noRedirectPolicy(_ *http.Request, _ []*http.Request) error {
return http.ErrUseLastResponse
}

// WebhookAuthorizer implements Authorizer by delegating access evaluation to an external HTTP service.
type WebhookAuthorizer struct {
client *http.Client
endpointURL string
}

// WebhookOption configures optional parameters for WebhookAuthorizer.
type WebhookOption func(*WebhookAuthorizer)

// WithHTTPClient sets a custom HTTP client for the webhook authorizer.
// The provided client must not be nil.
func WithHTTPClient(client *http.Client) WebhookOption {
return func(w *WebhookAuthorizer) {
w.client = client
}
}

// NewWebhookAuthorizer initializes a WebhookAuthorizer targeting the provided endpoint URL.
// It returns an error if endpointURL is empty, any option is nil, or the resolved HTTP client is nil.
func NewWebhookAuthorizer(endpointURL string, opts ...WebhookOption) (*WebhookAuthorizer, error) {
if endpointURL == "" {
return nil, errors.New("webhook endpoint URL must not be empty")
}
w := &WebhookAuthorizer{
endpointURL: endpointURL,
client: &http.Client{
Timeout: 10 * time.Second,
CheckRedirect: noRedirectPolicy,
},
}
for _, opt := range opts {
if opt == nil {
return nil, errors.New("webhook option must not be nil")
}
opt(w)
}
if w.client == nil {
return nil, errors.New("webhook HTTP client must not be nil")
}
// Enforce no-redirect policy on injected clients that do not set one.
if w.client.CheckRedirect == nil {
w.client.CheckRedirect = noRedirectPolicy
}
return w, nil
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved
}

// Authorize posts the authorization request to the webhook endpoint and decodes the result.
func (w *WebhookAuthorizer) Authorize(ctx context.Context, req *AuthRequest) (*AuthResult, error) {
if req == nil {
return nil, ErrUnauthorized
}

bodyBytes, err := json.Marshal(req)
if err != nil {
return nil, fmt.Errorf("%w: failed to marshal auth request: %w", ErrAuthorizerInternal, err)
}

httpReq, err := http.NewRequestWithContext(ctx, http.MethodPost, w.endpointURL, bytes.NewReader(bodyBytes))
if err != nil {
return nil, fmt.Errorf("%w: failed to create HTTP request: %w", ErrAuthorizerInternal, err)
}
httpReq.Header.Set("Content-Type", "application/json")

resp, err := w.client.Do(httpReq)
if err != nil {
return nil, fmt.Errorf("%w: webhook request failed: %w", ErrAuthorizerInternal, err)
Comment on lines +191 to +193

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | 🏗️ Heavy lift

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Map the target file and inspect relevant regions.
git ls-files src/libraries/go/lib/pkg/auth/authorizer.go
wc -l src/libraries/go/lib/pkg/auth/authorizer.go
cat -n src/libraries/go/lib/pkg/auth/authorizer.go | sed -n '1,260p'

# Search for tracing / otel usage in the auth package and nearby code.
rg -n "otel|trace\.|propagat|Inject|Span|Start\(" src/libraries/go/lib/pkg/auth src/libraries/go/lib/pkg -g'*.go'

Repository: NVIDIA/nvcf

Length of output: 50367


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the existing instrumented HTTP client pattern in the auth package.
cat -n src/libraries/go/lib/pkg/auth/auth_token_fetcher.go | sed -n '1,180p'

# Also inspect the generic HTTP client helper used elsewhere for tracing conventions.
cat -n src/libraries/go/lib/pkg/nvkit/clients/http_client.go | sed -n '220,260p'

Repository: NVIDIA/nvcf

Length of output: 8345


Instrument the webhook authorization request. Add an OTEL client span around w.client.Do, propagate W3C Trace Context, and mark transport/non-OK failures as errors to match the other outbound HTTP paths in src/libraries/go/lib/pkg/auth/authorizer.go:169-191.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/libraries/go/lib/pkg/auth/authorizer.go` around lines 169 - 171,
Instrument the outbound request in the authorizer flow around w.client.Do with
an OTEL client span and W3C Trace Context propagation, following the existing
outbound HTTP patterns in the same authorizer implementation. Mark transport
errors and non-OK HTTP responses on the span while preserving the current error
returns and response handling.

Sources: Coding guidelines, Path instructions

}
defer resp.Body.Close()

respBody, err := io.ReadAll(resp.Body)
if err != nil {
return nil, fmt.Errorf("%w: failed to read webhook response body: %w", ErrAuthorizerInternal, err)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Bound webhook response-body reads.

io.ReadAll lets a faulty or hostile webhook consume unbounded memory before status handling. Read through a size-limited reader and return ErrAuthorizerInternal when the limit is exceeded; add an oversized-response test.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/libraries/go/lib/pkg/auth/authorizer.go` around lines 175 - 177, Update
the response-body handling in the authorizer request flow to read through a
size-limited reader instead of calling io.ReadAll directly, and return
ErrAuthorizerInternal when the configured maximum is exceeded. Preserve existing
read-error wrapping and status handling for responses within the limit, and add
a test covering an oversized webhook response.

}

if resp.StatusCode == http.StatusUnauthorized {
return nil, ErrUnauthorized
}
Comment thread
shelleyshen-0 marked this conversation as resolved.
Resolved
if resp.StatusCode == http.StatusForbidden {
reason := "forbidden by policy"
if len(respBody) > 0 {
reason = string(respBody)
}
return &AuthResult{Allowed: false, Reason: reason}, nil
}
if resp.StatusCode != http.StatusOK {
return nil, fmt.Errorf("%w: unexpected status %d from webhook: %s", ErrAuthorizerInternal, resp.StatusCode, respBody)
}

var result AuthResult
if err := json.Unmarshal(respBody, &result); err != nil {
return nil, fmt.Errorf("%w: failed to decode webhook response: %w", ErrAuthorizerInternal, err)
}

return &result, nil
}
Loading
Loading