Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: statistics for orphaned tokens #7568

Merged
merged 10 commits into from
Jul 11, 2024
Merged

feat: statistics for orphaned tokens #7568

merged 10 commits into from
Jul 11, 2024

Conversation

Tymek
Copy link
Member

@Tymek Tymek commented Jul 10, 2024

About the changes

  1. Token Validation Updates:

    • Modified createTokenRowReducer to exclude tokens in v1 format from orphaned tokens. Those where created before the concept of project and environment and are always wildcard tokens.
  2. Metrics Monitoring:

    • Added countDeprecatedTokens to count:
      • orphanedTokens - tokens that cause issues, because of wildcard access
      • activeOrphanedTokens (within last 3 months)
      • legacyTokens - tokens in v1 format - metric added in case we would like to deprecate or migrate it couple of major version in the future
      • activeLegacyTokens (within last 3 months)

New Prometheus fields are:
orphaned_api_tokens_total, orphaned_api_tokens_active, legacy_api_tokens_total, legacy_api_tokens_active

  1. Feature Flags Adjustment:
    • Changed allowOrphanedWildcardTokens kill switch default to true.

Copy link

vercel bot commented Jul 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 10, 2024 4:08pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Jul 10, 2024 4:08pm

Copy link
Contributor

github-actions bot commented Jul 10, 2024

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

OpenSSF Scorecard

PackageVersionScoreDetails

Scanned Manifest Files

src/lib/db/api-token-store.ts Outdated Show resolved Hide resolved
@@ -82,7 +82,9 @@ beforeAll(async () => {
const config = createTestConfig({
getLogger,
experimental: {
flags: {},
flags: {
cleanApiTokenWhenOrphaned: true,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fun. Killswitch being disabled by default was masking that tests related to other flag don't pass.

@@ -227,6 +227,40 @@ export const instanceAdminStatsSchema = {
example:
'b023323477abb1eb145bebf3cdb30a1c2063e3edc1f7ae474ed8ed6c80de9a3b',
},
deprecatedTokens: {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we don't add this info to instance stats and only to metrics.ts then this part is not needed

Copy link
Member Author

@Tymek Tymek Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏻 I'll get metrics from store, in collectStaticCounters, skipping instance stats

name: 'test',
});
await stores.apiTokenStore.insert({
secret: '*:*.be44368985f7fb3237c584ef86f3d6bdada42ddbd63a019d26955178',
Copy link
Contributor

@kwasniew kwasniew Jul 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

which of the fields in the token are important. can we emphasize the fields that matter and hide then one that don't? e.g. if only secret counts const orphanedToken = token('*:*.be44368985f7fb3237c584ef86f3d6bdada42ddbd63a019d26955178')

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost all fields can count - for example tokens of ADMIN type can't be orphaned, because it doesn't have projects. At the same time it can be in v1 or v2 format. I'm providing a variety of possible correct tokens.

@Tymek Tymek merged commit b9c3d10 into main Jul 11, 2024
12 checks passed
@Tymek Tymek deleted the orphaned-tokens-stats branch July 11, 2024 09:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants