internal/testing: fake auth server #296
Conversation
This is a fake OAuth authentication server, for use in testing.
|
Do you want me to try to review? |
| Addr: authServerPort, | ||
| Handler: mux, | ||
| } | ||
| return server |
There was a problem hiding this comment.
Instead of a complete fake server, would it be simpler to have a this package provide a ServeMux? In the case where you want a server, you can use httptest:
srv := httptest.NewServer(fakeauth.NewMux())
This pushes the start-a-server logic off to httptest, which already handles it, and doesn't seem substantially less convenient to use in tests.
|
|
||
| func (s *FakeAuthServer) handleMetadata(w http.ResponseWriter, r *http.Request) { | ||
| metadata := map[string]any{ | ||
| "issuer": issuer, |
There was a problem hiding this comment.
Instead of hardcoding http://localhost:8080 as the issuer, should this use r.URL to find either the name or the port?
RFC 8414 says the issuer is present to prevent "mix-up" attacks, but that's probably not much of a concern for a test-only implementation. To be extra cautious, however, maybe this could set the issuer to "http://localhost:" + r.URL.Port().
|
|
||
| func (s *FakeAuthServer) handleMetadata(w http.ResponseWriter, r *http.Request) { | ||
| metadata := map[string]any{ | ||
| "issuer": issuer, |
There was a problem hiding this comment.
RFC 8414 requires most/all of the URLs here to have an "https" scheme. The test server uses HTTP. Not sure if this is going to be a problem in practice, but this might be something else using httptest would help with, since it handles setting up a test server with TLS support.
| if redirectURI == "" { | ||
| http.Error(w, "invalid_request", http.StatusBadRequest) | ||
| return | ||
| } |
There was a problem hiding this comment.
I might be missing something, but I believe redirect_uri is an optional parameter and only needs to be provided if a client has registered more than one redirect URI.
I'm going to guess that this fake auth server will only be used with clients that always provide a redirect_uri?
| if codeChallenge == "" || codeChallengeMethod != "S256" { | ||
| http.Error(w, "invalid_request", http.StatusBadRequest) | ||
| return | ||
| } |
There was a problem hiding this comment.
Should this check for the required client_id field? (It doesn't use it, but perhaps it would be useful to return an error to a client which fails to provide it?)
| return | ||
| } | ||
|
|
||
| authCode := "fake-auth-code-" + fmt.Sprintf("%d", time.Now().UnixNano()) |
There was a problem hiding this comment.
Unlikely though it is for a collision to happen here, perhaps this should include a counter or random component?
| r.ParseForm() | ||
| grantType := r.Form.Get("grant_type") | ||
| code := r.Form.Get("code") | ||
| redirectURI := r.Form.Get("redirect_uri") |
There was a problem hiding this comment.
redirect_uri was removed in OAuth 2.1:
https://www.ietf.org/archive/id/draft-ietf-oauth-v2-1-13.html#redirect-uri-in-token-request
If this needs to support OAuth 2.0 clients, I believe it should validate redirect_uri when present but not complain when it isn't present. I suspect there's more to be done to support OAuth 2.0, though--I haven't checked to see what else changes. If this is only supporting OAuth 2.1 (https://modelcontextprotocol.io/specification/draft/basic/authorization only mentions 2.1) then I think it can ignore redirect_uri.
This is a fake OAuth authentication server, for use in testing.