Skip to content

Commit c43ecad

Browse files
noctchillinmx-psi
andauthored
confighttp: Ensure that auth errors use provided error handler (#12714)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description OTLP/HTTP receiver does not encode authentication errors into a `Status` object, and as such not expose a developer facing error message in the `Status.mesage` field. - https://opentelemetry.io/docs/specs/otlp/#failures-1 Resolves this issue by handling authentication errors with the same error handler provided to the Server. <!-- Issue number if applicable --> #### Link to tracking issue Fixes #12666 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests for handling errors with and without a server error handler <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Pablo Baeyens <[email protected]>
1 parent 564818f commit c43ecad

File tree

3 files changed

+72
-4
lines changed

3 files changed

+72
-4
lines changed
+25
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver)
7+
component: confighttp
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: "Ensure http authentication server failures are handled by the provided error handler"
11+
12+
# One or more tracking issues or pull requests related to the change
13+
issues: [12666]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# Optional: The change log or logs in which this entry should be included.
21+
# e.g. '[user]' or '[user, api]'
22+
# Include 'user' if the change is relevant to end users.
23+
# Include 'api' if there is a change to a library API.
24+
# Default: '[user]'
25+
change_logs: [user]

config/confighttp/confighttp.go

+8-3
Original file line numberDiff line numberDiff line change
@@ -441,7 +441,7 @@ func (hss *ServerConfig) ToServer(_ context.Context, host component.Host, settin
441441
return nil, err
442442
}
443443

444-
handler = authInterceptor(handler, server, hss.Auth.RequestParameters)
444+
handler = authInterceptor(handler, server, hss.Auth.RequestParameters, serverOpts)
445445
}
446446

447447
if hss.CORS != nil && len(hss.CORS.AllowedOrigins) > 0 {
@@ -537,7 +537,7 @@ func NewDefaultCORSConfig() *CORSConfig {
537537
return &CORSConfig{}
538538
}
539539

540-
func authInterceptor(next http.Handler, server extensionauth.Server, requestParams []string) http.Handler {
540+
func authInterceptor(next http.Handler, server extensionauth.Server, requestParams []string, serverOpts *internal.ToServerOptions) http.Handler {
541541
return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
542542
sources := r.Header
543543
query := r.URL.Query()
@@ -548,7 +548,12 @@ func authInterceptor(next http.Handler, server extensionauth.Server, requestPara
548548
}
549549
ctx, err := server.Authenticate(r.Context(), sources)
550550
if err != nil {
551-
http.Error(w, http.StatusText(http.StatusUnauthorized), http.StatusUnauthorized)
551+
if serverOpts.ErrHandler != nil {
552+
serverOpts.ErrHandler(w, r, err.Error(), http.StatusUnauthorized)
553+
} else {
554+
http.Error(w, err.Error(), http.StatusUnauthorized)
555+
}
556+
552557
return
553558
}
554559

config/confighttp/confighttp_test.go

+39-1
Original file line numberDiff line numberDiff line change
@@ -1212,7 +1212,7 @@ func TestFailedServerAuth(t *testing.T) {
12121212
host := &mockHost{
12131213
ext: map[component.ID]component.Component{
12141214
mockID: newMockAuthServer(func(ctx context.Context, _ map[string][]string) (context.Context, error) {
1215-
return ctx, errors.New("Settings failed")
1215+
return ctx, errors.New("invalid authorization")
12161216
}),
12171217
},
12181218
}
@@ -1229,6 +1229,44 @@ func TestFailedServerAuth(t *testing.T) {
12291229
assert.Equal(t, fmt.Sprintf("%v %s", http.StatusUnauthorized, http.StatusText(http.StatusUnauthorized)), response.Result().Status)
12301230
}
12311231

1232+
func TestFailedServerAuthWithErrorHandler(t *testing.T) {
1233+
// prepare
1234+
hss := ServerConfig{
1235+
Endpoint: "localhost:0",
1236+
Auth: &AuthConfig{
1237+
Authentication: configauth.Authentication{
1238+
AuthenticatorID: mockID,
1239+
},
1240+
},
1241+
}
1242+
host := &mockHost{
1243+
ext: map[component.ID]component.Component{
1244+
mockID: newMockAuthServer(func(ctx context.Context, _ map[string][]string) (context.Context, error) {
1245+
return ctx, errors.New("invalid authorization")
1246+
}),
1247+
},
1248+
}
1249+
1250+
eh := func(w http.ResponseWriter, _ *http.Request, err string, statusCode int) {
1251+
assert.Equal(t, http.StatusUnauthorized, statusCode)
1252+
// custom error handler uses real error string
1253+
assert.Equal(t, "invalid authorization", err)
1254+
// custom error handler changes returned status code
1255+
http.Error(w, err, http.StatusInternalServerError)
1256+
}
1257+
1258+
srv, err := hss.ToServer(context.Background(), host, componenttest.NewNopTelemetrySettings(), http.HandlerFunc(func(http.ResponseWriter, *http.Request) {}), WithErrorHandler(eh))
1259+
require.NoError(t, err)
1260+
1261+
// tt
1262+
response := &httptest.ResponseRecorder{}
1263+
srv.Handler.ServeHTTP(response, httptest.NewRequest(http.MethodGet, "/", nil))
1264+
1265+
// verify
1266+
assert.Equal(t, http.StatusInternalServerError, response.Result().StatusCode)
1267+
assert.Equal(t, fmt.Sprintf("%v %s", http.StatusInternalServerError, http.StatusText(http.StatusInternalServerError)), response.Result().Status)
1268+
}
1269+
12321270
func TestServerWithErrorHandler(t *testing.T) {
12331271
// prepare
12341272
hss := ServerConfig{

0 commit comments

Comments
 (0)