[v2.8] [backport] AzureAD support #363
Conversation
bump rancher added dashboard implementation
updated login request removed unused import removed prompt flag
go mod fix
- added getClient func to create once the HTTP client with the same TLS configuration. - added tests for the getAuthProviders func
enrichman
force-pushed
the
44282-azuread-support
branch
from
d95eb33
to
dbf5eed
Compare
samjustus
requested review from
pmatseykanets
and removed request for
andreas-kupries and
JonCrowther
| i++ | ||
| data := gjson.GetBytes(response, "data").Array() | ||
|
|
||
| supportedProviders := []TypedProvider{} |
There was a problem hiding this comment.
the nil slice is the preferred style
https://go.dev/wiki/CodeReviewComments#declaring-empty-slices
I'll be honest, this surprises me, and I am guilty of not doing this :-)
|
|
||
| err = json.Unmarshal([]byte(provider.Raw), typedProvider) | ||
| if err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
We could maybe wrap this with something that says "attempting to decode the auth provider of type %s", otherwise these sorts of errors are quite generic (and thus opaque).
| if err != nil { | ||
| return nil, err | ||
| if !gjson.ValidBytes(response) { | ||
| return nil, errors.New("invalid JSON input") |
There was a problem hiding this comment.
How about "response" here rather than "input"? and could we include the authProviderURL which might point to why?
| func Test_getAuthProviders(t *testing.T) { | ||
|
|
||
| setupServer := func(response string) *httptest.Server { | ||
| return httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
The servers created by this aren't .Closed() ?
Probably won't impact in the scope of tests.
Reading the code tho', it feels like you could create and either defer .Close() or t.Cleanup() and close?
Ref:
Backport of #346