Notify users about expiring git PATs and expose PATs in admin panel (#33802)
* Allow admin access to user PATs * Tests for new screen in admin panel * Adding error for invalid token and way to parse error for OAuth 2 * Git bridge handles expired PAT * Script for alerting on close to expiry and expired git tokens * Refactoring and simplifying * Updating email templates to match agreed docs * tweak to email subject to include Overleaf * Allowing dry run in scripts and general tidy up * removing redundant tests and dry running script * Fixing CI errors * Adding new tab to admin test expectation * Address PR feedback on oauth2-server changes - Replace ad-hoc overleafErrorCode prop with a TokenExpiredError subclass - Collapse listTokens/listTokensForAdmin into a single hook Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * Adding cron definitions for alerting on expiring git pat --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> GitOrigin-RevId: 69b9fd901a201592a580c69abe7bd7d603e85d3a
This commit is contained in:
committed by
Copybot
co-authored by
Claude Opus 4.7
parent
a553a8390d
commit
e53c6f2aea
@@ -730,6 +730,83 @@ describe('AuthenticationController', function () {
|
||||
ctx.next.should.have.not.been.calledOnce
|
||||
})
|
||||
})
|
||||
|
||||
describe('error_code classification', function () {
|
||||
// The classifier reads err.name (RFC-standard snake_case from
|
||||
// @node-oauth/oauth2-server) plus an overleafErrorCode marker we
|
||||
// attach ourselves. No reliance on err.message — that keeps the
|
||||
// classification immune to library description changes.
|
||||
async function runMiddlewareWithError(ctx, err) {
|
||||
await new Promise(resolve => {
|
||||
ctx.res.json.callsFake(() => resolve())
|
||||
ctx.Oauth2Server.server.authenticate.rejects(err)
|
||||
ctx.middleware(ctx.req, ctx.res, ctx.next)
|
||||
})
|
||||
}
|
||||
|
||||
it('returns "token_expired" when Oauth2ServerModel marks the error', async function (ctx) {
|
||||
await runMiddlewareWithError(ctx, {
|
||||
code: 401,
|
||||
name: 'invalid_token',
|
||||
overleafErrorCode: 'token_expired',
|
||||
})
|
||||
ctx.res.json.should.have.been.calledWithMatch({
|
||||
error_code: 'token_expired',
|
||||
})
|
||||
})
|
||||
|
||||
it('returns "token_invalid" for an invalid_token error without a marker', async function (ctx) {
|
||||
await runMiddlewareWithError(ctx, {
|
||||
code: 401,
|
||||
name: 'invalid_token',
|
||||
})
|
||||
ctx.res.json.should.have.been.calledWithMatch({
|
||||
error_code: 'token_invalid',
|
||||
})
|
||||
})
|
||||
|
||||
it('returns "token_malformed" for a malformed authorization header', async function (ctx) {
|
||||
await runMiddlewareWithError(ctx, {
|
||||
code: 400,
|
||||
name: 'invalid_request',
|
||||
message: 'Invalid request: malformed authorization header',
|
||||
})
|
||||
ctx.res.json.should.have.been.calledWithMatch({
|
||||
error_code: 'token_malformed',
|
||||
})
|
||||
})
|
||||
|
||||
it('returns "invalid_request" for any other invalid_request error', async function (ctx) {
|
||||
await runMiddlewareWithError(ctx, {
|
||||
code: 400,
|
||||
name: 'invalid_request',
|
||||
message: 'Invalid request: something else',
|
||||
})
|
||||
ctx.res.json.should.have.been.calledWithMatch({
|
||||
error_code: 'invalid_request',
|
||||
})
|
||||
})
|
||||
|
||||
it('returns "insufficient_scope" for an insufficient_scope error', async function (ctx) {
|
||||
await runMiddlewareWithError(ctx, {
|
||||
code: 403,
|
||||
name: 'insufficient_scope',
|
||||
})
|
||||
ctx.res.json.should.have.been.calledWithMatch({
|
||||
error_code: 'insufficient_scope',
|
||||
})
|
||||
})
|
||||
|
||||
it('returns "unauthorized_request" for an unauthorized_request error', async function (ctx) {
|
||||
await runMiddlewareWithError(ctx, {
|
||||
code: 401,
|
||||
name: 'unauthorized_request',
|
||||
})
|
||||
ctx.res.json.should.have.been.calledWithMatch({
|
||||
error_code: 'unauthorized_request',
|
||||
})
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('requireGlobalLogin', function () {
|
||||
|
||||
@@ -173,6 +173,50 @@ describe('EmailBuilder', function () {
|
||||
})
|
||||
})
|
||||
|
||||
describe('gitTokenExpiringSoon', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.opts = {
|
||||
to: 'user@example.com',
|
||||
}
|
||||
ctx.email = ctx.EmailBuilder.buildEmail('gitTokenExpiringSoon', ctx.opts)
|
||||
})
|
||||
|
||||
it('should render html, text, and subject without undefined', function (ctx) {
|
||||
expect(ctx.email.html).to.not.be.undefined
|
||||
expect(ctx.email.text).to.not.be.undefined
|
||||
expect(ctx.email.subject).to.not.be.undefined
|
||||
ctx.email.html.indexOf('undefined').should.equal(-1)
|
||||
ctx.email.text.indexOf('undefined').should.equal(-1)
|
||||
ctx.email.subject.indexOf('undefined').should.equal(-1)
|
||||
})
|
||||
|
||||
it('should link the CTA to user settings', function (ctx) {
|
||||
ctx.email.text.should.contain(`${ctx.settings.siteUrl}/user/settings`)
|
||||
})
|
||||
})
|
||||
|
||||
describe('gitTokenExpired', function () {
|
||||
beforeEach(function (ctx) {
|
||||
ctx.opts = {
|
||||
to: 'user@example.com',
|
||||
}
|
||||
ctx.email = ctx.EmailBuilder.buildEmail('gitTokenExpired', ctx.opts)
|
||||
})
|
||||
|
||||
it('should render html, text, and subject without undefined', function (ctx) {
|
||||
expect(ctx.email.html).to.not.be.undefined
|
||||
expect(ctx.email.text).to.not.be.undefined
|
||||
expect(ctx.email.subject).to.not.be.undefined
|
||||
ctx.email.html.indexOf('undefined').should.equal(-1)
|
||||
ctx.email.text.indexOf('undefined').should.equal(-1)
|
||||
ctx.email.subject.indexOf('undefined').should.equal(-1)
|
||||
})
|
||||
|
||||
it('should link the CTA to user settings', function (ctx) {
|
||||
ctx.email.text.should.contain(`${ctx.settings.siteUrl}/user/settings`)
|
||||
})
|
||||
})
|
||||
|
||||
describe('ctaTemplate', function () {
|
||||
describe('missing required content', function () {
|
||||
const content = {
|
||||
|
||||
@@ -0,0 +1,213 @@
|
||||
import { vi, expect } from 'vitest'
|
||||
import sinon from 'sinon'
|
||||
|
||||
const SCRIPT_PATH = '../../../../scripts/oauth/notify_expiring_tokens.mjs'
|
||||
|
||||
describe('notify_expiring_tokens', function () {
|
||||
beforeEach(async function (ctx) {
|
||||
ctx.userEmail = 'user@example.com'
|
||||
|
||||
ctx.User = {
|
||||
findOne: sinon.stub().returns({
|
||||
exec: sinon.stub().resolves({ email: ctx.userEmail }),
|
||||
}),
|
||||
}
|
||||
|
||||
ctx.collection = {
|
||||
cursor: [],
|
||||
find: sinon.stub().callsFake(() => ({
|
||||
[Symbol.asyncIterator]: async function* () {
|
||||
for (const t of ctx.collection.cursor) yield t
|
||||
},
|
||||
})),
|
||||
updateOne: sinon.stub().resolves({ modifiedCount: 1 }),
|
||||
}
|
||||
|
||||
ctx.EmailHandler = {
|
||||
promises: {
|
||||
sendEmail: sinon.stub().resolves(),
|
||||
},
|
||||
}
|
||||
|
||||
vi.doMock('../../../../app/src/infrastructure/mongodb.mjs', () => ({
|
||||
db: { oauthAccessTokens: ctx.collection },
|
||||
READ_PREFERENCE_SECONDARY: 'secondary',
|
||||
}))
|
||||
|
||||
vi.doMock('../../../../app/src/models/User.mjs', () => ({
|
||||
User: ctx.User,
|
||||
}))
|
||||
|
||||
vi.doMock('../../../../app/src/Features/Email/EmailHandler.mjs', () => ({
|
||||
default: ctx.EmailHandler,
|
||||
}))
|
||||
|
||||
vi.doMock('@overleaf/settings', () => ({
|
||||
default: {
|
||||
personalAccessTokens: { expiry: { warningWindowDays: 2 } },
|
||||
},
|
||||
}))
|
||||
|
||||
vi.doMock('../../../../scripts/lib/ScriptRunner.mjs', () => ({
|
||||
scriptRunner: async fn => fn(),
|
||||
}))
|
||||
|
||||
ctx.script = await import(SCRIPT_PATH)
|
||||
})
|
||||
|
||||
describe('notifyOwner', function () {
|
||||
it('returns true and sets lastNotifiedAt on successful send', async function (ctx) {
|
||||
const token = {
|
||||
_id: 'tok-1',
|
||||
user_id: 'user-1',
|
||||
accessTokenExpiresAt: new Date('2026-05-20T00:00:00Z'),
|
||||
}
|
||||
const ok = await ctx.script.notifyOwner({
|
||||
token,
|
||||
kind: 'warning',
|
||||
template: 'gitTokenExpiringSoon',
|
||||
})
|
||||
expect(ok).to.equal(true)
|
||||
expect(ctx.EmailHandler.promises.sendEmail).to.have.been.calledWith(
|
||||
'gitTokenExpiringSoon',
|
||||
sinon.match({ to: ctx.userEmail })
|
||||
)
|
||||
expect(ctx.collection.updateOne).to.have.been.calledOnce
|
||||
const update = ctx.collection.updateOne.firstCall.args[1]
|
||||
expect(update.$set).to.have.property('lastNotifiedAt.warning')
|
||||
})
|
||||
|
||||
it('returns false and does NOT set the marker if EmailHandler rejects', async function (ctx) {
|
||||
ctx.EmailHandler.promises.sendEmail.rejects(new Error('SMTP down'))
|
||||
const token = {
|
||||
_id: 'tok-2',
|
||||
user_id: 'user-2',
|
||||
accessTokenExpiresAt: new Date('2026-05-20T00:00:00Z'),
|
||||
}
|
||||
const ok = await ctx.script.notifyOwner({
|
||||
token,
|
||||
kind: 'expired',
|
||||
template: 'gitTokenExpired',
|
||||
})
|
||||
expect(ok).to.equal(false)
|
||||
expect(ctx.collection.updateOne).to.not.have.been.called
|
||||
})
|
||||
|
||||
it('returns false and skips when the owner has no email', async function (ctx) {
|
||||
ctx.User.findOne.returns({
|
||||
exec: sinon.stub().resolves(null),
|
||||
})
|
||||
const token = {
|
||||
_id: 'tok-3',
|
||||
user_id: 'user-3',
|
||||
accessTokenExpiresAt: new Date('2026-05-20T00:00:00Z'),
|
||||
}
|
||||
const ok = await ctx.script.notifyOwner({
|
||||
token,
|
||||
kind: 'warning',
|
||||
template: 'gitTokenExpiringSoon',
|
||||
})
|
||||
expect(ok).to.equal(false)
|
||||
expect(ctx.EmailHandler.promises.sendEmail).to.not.have.been.called
|
||||
expect(ctx.collection.updateOne).to.not.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('processBucket', function () {
|
||||
it('iterates all matching tokens and counts successful sends', async function (ctx) {
|
||||
ctx.collection.cursor = [
|
||||
{
|
||||
_id: 't1',
|
||||
user_id: 'u1',
|
||||
accessTokenExpiresAt: new Date('2026-05-20T00:00:00Z'),
|
||||
},
|
||||
{
|
||||
_id: 't2',
|
||||
user_id: 'u2',
|
||||
accessTokenExpiresAt: new Date('2026-05-21T00:00:00Z'),
|
||||
},
|
||||
]
|
||||
const count = await ctx.script.processBucket({
|
||||
kind: 'warning',
|
||||
template: 'gitTokenExpiringSoon',
|
||||
query: { type: 'pat' },
|
||||
})
|
||||
expect(count).to.equal(2)
|
||||
expect(ctx.EmailHandler.promises.sendEmail).to.have.been.calledTwice
|
||||
expect(ctx.collection.updateOne).to.have.been.calledTwice
|
||||
})
|
||||
|
||||
it('continues processing remaining tokens after one send fails', async function (ctx) {
|
||||
ctx.collection.cursor = [
|
||||
{
|
||||
_id: 't1',
|
||||
user_id: 'u1',
|
||||
accessTokenExpiresAt: new Date('2026-05-20T00:00:00Z'),
|
||||
},
|
||||
{
|
||||
_id: 't2',
|
||||
user_id: 'u2',
|
||||
accessTokenExpiresAt: new Date('2026-05-21T00:00:00Z'),
|
||||
},
|
||||
]
|
||||
ctx.EmailHandler.promises.sendEmail
|
||||
.onFirstCall()
|
||||
.rejects(new Error('transient'))
|
||||
.onSecondCall()
|
||||
.resolves()
|
||||
|
||||
const count = await ctx.script.processBucket({
|
||||
kind: 'expired',
|
||||
template: 'gitTokenExpired',
|
||||
query: { type: 'pat' },
|
||||
})
|
||||
expect(count).to.equal(1)
|
||||
expect(ctx.collection.updateOne).to.have.been.calledOnce
|
||||
})
|
||||
|
||||
it('returns 0 when no tokens match', async function (ctx) {
|
||||
ctx.collection.cursor = []
|
||||
const count = await ctx.script.processBucket({
|
||||
kind: 'warning',
|
||||
template: 'gitTokenExpiringSoon',
|
||||
query: { type: 'pat' },
|
||||
})
|
||||
expect(count).to.equal(0)
|
||||
expect(ctx.EmailHandler.promises.sendEmail).to.not.have.been.called
|
||||
})
|
||||
})
|
||||
|
||||
describe('main query construction', function () {
|
||||
it('queries the warning bucket within the configured window and skips already-warned tokens', async function (ctx) {
|
||||
ctx.collection.cursor = []
|
||||
await ctx.script.main()
|
||||
|
||||
const warningCall = ctx.collection.find
|
||||
.getCalls()
|
||||
.find(c => c.args[0]['lastNotifiedAt.warning'])
|
||||
expect(warningCall).to.exist
|
||||
const q = warningCall.args[0]
|
||||
expect(q.type).to.equal('pat')
|
||||
expect(q['lastNotifiedAt.warning']).to.deep.equal({ $exists: false })
|
||||
expect(q.accessTokenExpiresAt.$gt).to.be.instanceOf(Date)
|
||||
expect(q.accessTokenExpiresAt.$lte).to.be.instanceOf(Date)
|
||||
const horizonMs =
|
||||
q.accessTokenExpiresAt.$lte.getTime() -
|
||||
q.accessTokenExpiresAt.$gt.getTime()
|
||||
expect(horizonMs).to.equal(2 * 24 * 60 * 60 * 1000)
|
||||
})
|
||||
|
||||
it('queries the expired bucket and excludes suppressed tokens', async function (ctx) {
|
||||
ctx.collection.cursor = []
|
||||
await ctx.script.main()
|
||||
|
||||
const expiredCall = ctx.collection.find
|
||||
.getCalls()
|
||||
.find(c => c.args[0]['lastNotifiedAt.expired'])
|
||||
expect(expiredCall).to.exist
|
||||
const q = expiredCall.args[0]
|
||||
expect(q['lastNotifiedAt.expired']).to.deep.equal({ $exists: false })
|
||||
expect(q.notificationsSuppressedAt).to.deep.equal({ $exists: false })
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user