oidc: accept both the app's public client and the server's confidential one
server-test / test (push) Successful in 37s
server-test / test (push) Successful in 37s
Explaining public vs confidential clients surfaced a gap in my own design: I had
assumed a single client id, but there are two clients here with genuinely
different properties.
the Android app public + PKCE, because an APK cannot keep a secret
the admin UI confidential, because the server can keep one in
/etc/echolot-server.env and weakening it to public buys
nothing
So the audience check now accepts either registered client id - and only those
two. "Any client of this issuer" would let every other application registered
with the same IdP authenticate here, which is the entire reason the check
exists. Either id alone is enough to enable sign-in, since an operator may
register only the app or only the admin UI.
The profile advertises the *app's* client id, since that is what a phone should
authorize as.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
89a5ff9139
commit
80d2092f1b
@@ -115,7 +115,9 @@ func (i *testIdP) claims(extra map[string]any) map[string]any {
|
||||
}
|
||||
|
||||
func verifier(i *testIdP, adminGroup string) *Verifier {
|
||||
return New(Config{Issuer: i.URL, ClientID: "echolot", AdminGroup: adminGroup}, i.Client())
|
||||
return New(Config{
|
||||
Issuer: i.URL, ClientID: "echolot", AppClientID: "echolot-app", AdminGroup: adminGroup,
|
||||
}, i.Client())
|
||||
}
|
||||
|
||||
func TestAcceptsAGenuineToken(t *testing.T) {
|
||||
@@ -286,3 +288,38 @@ func TestDisabledWithoutConfiguration(t *testing.T) {
|
||||
t.Fatalf("want ErrDisabled, got %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Two clients, because the phone and the admin UI have different properties: an APK cannot keep a
|
||||
// secret (public + PKCE) while the server can (confidential). Both must be accepted — but only
|
||||
// those two. "Any client of this issuer" would let every other application registered with the
|
||||
// same IdP authenticate here, which is the whole reason the audience check exists.
|
||||
func TestBothRegisteredClientsAreAccepted(t *testing.T) {
|
||||
idp := newIdP(t)
|
||||
v := verifier(idp, "")
|
||||
|
||||
for _, aud := range []any{"echolot", "echolot-app", []string{"echolot-app", "other"}} {
|
||||
tok := idp.sign(t, "RS256", "rsa-1", idp.claims(map[string]any{"aud": aud}))
|
||||
if _, err := v.Verify(context.Background(), tok); err != nil {
|
||||
t.Errorf("aud %v was refused: %v", aud, err)
|
||||
}
|
||||
}
|
||||
// A third application at the same issuer is still not us.
|
||||
tok := idp.sign(t, "RS256", "rsa-1", idp.claims(map[string]any{"aud": "someone-elses-app"}))
|
||||
if _, err := v.Verify(context.Background(), tok); !errors.Is(err, ErrClaims) {
|
||||
t.Fatalf("a third client's token was accepted: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// Either client id alone is enough to make sign-in usable: an operator may register only the app
|
||||
// (no admin UI login) or only the server.
|
||||
func TestEitherClientIDAloneEnablesSignIn(t *testing.T) {
|
||||
if !(Config{Issuer: "https://i", ClientID: "a"}).Enabled() {
|
||||
t.Error("a server-only configuration was reported disabled")
|
||||
}
|
||||
if !(Config{Issuer: "https://i", AppClientID: "b"}).Enabled() {
|
||||
t.Error("an app-only configuration was reported disabled")
|
||||
}
|
||||
if (Config{Issuer: "https://i"}).Enabled() {
|
||||
t.Error("an issuer with no client at all was reported enabled")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user