fix: API key expiration dropdown never persists — date transform nulls relative expiration strings - #43
Merged
Conversation
The api-keys console assigns the expiration dropdown's relative strings
('never', 'immediately', 'in 1 hour', ...) directly to expires_at, but the
attribute was declared @attr('date') and Ember Data's date transform
serializes any non-Date value to null — so every option reached the API as
expires_at: null and no expiry was ever persisted. The roll path was
unaffected because it reads the raw attribute and PATCHes it outside the
serializer.
Add an `expiration` transform that passes strings through for the server
to resolve (core-api's ApiCredential::setExpiresAtAttribute owns the
relative-time parsing), serializes real Dates to ISO strings, and
deserializes datetimes like the date transform so the display path
(expiresAt computed, list column) keeps receiving Date instances.
Also make the engine's own test suite runnable, since nothing covered this
path (or ran at all):
- eager-load the engine for its own `ember test` runs (fleetops pattern)
so the dummy app can resolve engine modules
- add @ember/legacy-built-in-components so ember-engines' LinkToExternal
has an extensible base class under ember-source 5.4
- add prismjs and skip the ember-prism component config for self-test
builds, where ember-cli-node-assets registers imports without funneling
the files and the vendor concat fails
- regression tests: relative strings survive model.serialize(), Dates
serialize to ISO, datetimes deserialize to Dates for display
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Setting an expiry on an API credential in the developers console appears to succeed, but
api_credentials.expires_atstaysNULL. Every dropdown option behaves this way, includingimmediately, and the row saves without error — so revoking a key via expiry silently does nothing.Root cause: the edit/create modal assigns the dropdown's relative strings (
'never','immediately','in 1 hour', …) straight ontoexpires_at(controllers/api-keys/index.js:320-322), but the model declared@attr('date') expires_at. Ember Data'sdatetransform serializes anything that isn't a validDateinstance tonull, so every option reached the API asexpires_at: null.'never'mapping toNULLis correct by coincidence, which is probably why this survived so long.The roll path was never affected: it reads the raw attribute and sends it via
fetch.patch(..., { expiration: apiKey.get('expires_at') }), bypassing the serializer. That asymmetry confirmed the serializer as the culprit.The server is the right owner of relative-time parsing — core-api's
ApiCredential::setExpiresAtAttribute()is written to accept exactly these strings — so the client must deliver them untouched.Fix
New
expirationtransform, andexpires_atbecomes@attr('expiration'):Dates → ISO string; anything else →null.datetransform, so the display path (expiresAtcomputed withisValidDate/formatDate, the list's Expiry column) keeps receivingDateinstances for API-returned datetimes.Alternatives rejected
@attr('string')— breaks the display path:expiresAtcallsisValidDate()/formatDate()on the value, and API responses would surface as raw strings ("Never" shown for keys that do expire). Every read site would need re-working.Dateclient-side — moves expiry math into the console where it can drift from the server'sstrtotimeinterpretation, and changesimmediately's meaning from save-time to select-time.serializeAttributeoverride — equivalent behavior, but the transform is declarative, scoped to the attribute, and reusable.Verification
dev-engine@0.2.14, serializing anapi-credentialthrough the real store producedexpires_at: nullfor all six dropdown options. With this fix, all six strings pass through verbatim and aDateserializes to ISO.setExpiresAtAttributemapsnever→NULL,immediately→now,in 1 hour/in 24 hours/in 3 days/in 7 days→correct future datetimes, ISO datetime→parsed. So the full path is: console sends'in 24 hours'→ DB gets the correct datetime.immediatelyhad a second, stacked defect server-side (strict<inExpirable::hasExpired()); fix(auth): api credentials survived revocation and creator removal core-api#246 makes it inclusive (<=), agreeing withExpiryScope. Confirmed merged in core-apimain. Both this PR and #246 must ship forimmediatelyto reliably kill a key at the boundary instant.Regression coverage (and making the suite runnable at all)
Nothing tested this path — and the engine's suite couldn't run:
ember testdied before executing a single test (ember-engines'LinkToExternalextends a non-extensibleLinkTounder ember-source 5.4, and the lazy engine's modules aren't resolvable from the dummy app). CI never runsember testeither. This PR makes the suite runnable:ember testruns only (same pattern as the fleetops engine'sindex.js); hosts always get the lazy engine@ember/legacy-built-in-components(devDependency) soLinkToExternalhas an extensible base — the console already depends on it, which is why the console buildsprismjs(devDependency) and skip the ember-prism component config for self-test builds, whereember-cli-node-assetsregisters the imports without funneling the files in and the vendor concat fails with ENOENTNew tests (all fail on the old
@attr('date')code — verified by reverting the attr as a control):tests/unit/transforms/expiration-test.js— string passthrough, Date→ISO, null/invalid handling, deserialization (incl.+0000offsets)tests/unit/models/api-credential-test.js—model.serialize().expires_atpreserves each relative option end-to-end through the real serializer chain; Date→ISO; datetime deserializes to aDateand formats via theexpiresAtcomputedNote: with the suite now actually executing, ~44 pre-existing tests fail on missing dummy-app service stubs (
hostRouter,intl,universe, …) — test debt that predates this PR, tracked separately.Downstream
No
api-credentialmodel override exists in this workspace's host apps (checked the console and the fliit package). IfFliitAU/fliit-extensionshipped one downstream, it should be removable once this lands and the package is released.Downstream reports: FliitAU/fliit-extension#2217, FliitAU/fliit-extension#2212. Related: fleetbase/core-api#246