The login package contradicts the specification, fails its tests, and is unused
- Dominant language
- Rust
- Stars
- 3
- Forks
- 0
- Avg merge
- 1d 42m
- Merged PRs (30d)
- 11
Description
The `login/` package is a separate Go module implementing challenge generation, login request signing and verification, bearer token issue and verification, and HTTP middleware. It is in three states at once that should be resolved together: it contradicts the specification, it fails its own tests, and nothing uses it.
## It keys users by thumbprint, which the specification does not permit
`login.go` looks a key up by thumbprint and returns that thumbprint as the user identity:
```go
KeyLookup func(tmb coz.B64) (*coz.Key, error)
...
key, err = cfg.KeyLookup(cz.Parsed.Tmb)
...
return string(cz.Parsed.Tmb), nil // Use tmb as userID
```
Its login request payload carries no principal field at all.
`SPEC.md`'s Sharing Keys section says:
> Nothing in Cyphr stops various principals from sharing keys, as long as genesis does not result in the same PG. Any set of keys that has not been revoked may be used to create a new PG, this includes reusing keys from the source principal.
If one key may be active in several principals, a thumbprint does not identify a principal, so a thumbprint-keyed lookup is ambiguous by construction. The Rust server resolves this by taking a claimed principal from the request and verifying the signing key is active in it, and there is a test asserting the distinction is real: the same key and signature are accepted for the principal the key is active in and rejected for a different claimed principal that does not have it.
Note that the specification's own login section does not define a principal field for the request either, so the server's approach is an extension rather than an implementation of it. That gap is worth resolving in the specification independently of this package.
## It fails its own tests
```
cd login && go vet ./... # exit 0
cd login && go test ./... # exit 1
--- FAIL: TestLoginFlow login_test.go:128: ID mismatch: "" != "'.&\xac..."
--- FAIL: TestInvalidTimestamp
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV] ... login_test.go:188
```
It compiles and vets clean, which is why the failure has gone unnoticed. Two further signs it was left mid-draft: an example whose expected output is the literal placeholder `[some 43-char base64 tmb]`, and a comment reading `... (TestInvalidTimestamp and TestRevokedKey remain the same, just update NewKey calls)`.
## Nothing builds or imports it
```
git grep "coz_login\|coze_go_x/login" -- . ':!login' # no matches
git grep -rn "login" -- .github # no matches
```
The Go continuous-integration jobs all declare `working-directory: go`, so this module is outside them. It has been untouched since January apart from a formatting pass and a rename.
## What this needs
A decision rather than a patch, because the three states interact. If the package is intended to be finished, the thumbprint identity model has to change first, and the tests and continuous integration follow. If it is superseded by the Rust server, it should be removed rather than left as a compiling, non-building, failing module that a reader may reasonably take as current. Leaving it as it is has a specific cost: it is the most complete-looking answer in this repository to "how does login work", and it is both broken and inconsistent with the specification.
## Reproduce
```
cd login && go vet ./... ; echo "VET=$?"
cd login && go test ./... ; echo "TEST=$?"
grep -n "Use tmb as userID" login/login.go
grep -n -A3 "Sharing Keys" SPEC.md
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Start with login/login.go and login/login_test.go, then compare the package with the Sharing Keys section in SPEC.md and the Rust server's principal-handling test. Run go vet ./... and go test ./... from login, and inspect the .github workflows and repository imports. Done requires deciding whether to finish the package with a specification-consistent identity model or remove it if the Rust server supersedes it.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- go, rust
- Domain
- authentication, backend-api-design
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Quiet
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100