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

fix: Throw error on user disabled and check revoked set true #1401

Merged
merged 18 commits into from
Aug 16, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 29 additions & 15 deletions src/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,13 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI

/**
* Verifies a JWT auth token. Returns a Promise with the tokens claims. Rejects
* the promise if the token could not be verified. If checkRevoked is set to true,
* verifies if the session corresponding to the ID token was revoked. If the corresponding
* user's session was invalidated, an auth/id-token-revoked error is thrown. If not specified
* the check is not applied.
* the promise if the token could not be verified.
* If checkRevoked is set to true, first verifies whether the corresponding userinfo.disabled
* is true:
xil222 marked this conversation as resolved.
Show resolved Hide resolved
* If yes, an auth/user-disabled error is thrown.
* If no, verifies if the session corresponding to the ID token was revoked.
* If the corresponding user's session was invalidated, an auth/id-token-revoked error is thrown.
* If not specified the check is not applied.
*
* @param {string} idToken The JWT to verify.
* @param {boolean=} checkRevoked Whether to check if the ID token is revoked.
Expand All @@ -121,7 +124,7 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
.then((decodedIdToken: DecodedIdToken) => {
// Whether to check if the token was revoked.
if (checkRevoked || isEmulator) {
return this.verifyDecodedJWTNotRevoked(
return this.verifyDecodedJWTNotRevokedOrDisabled(
decodedIdToken,
AuthClientErrorCode.ID_TOKEN_REVOKED);
}
Expand Down Expand Up @@ -507,10 +510,14 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI

/**
* Verifies a Firebase session cookie. Returns a Promise with the tokens claims. Rejects
* the promise if the token could not be verified. If checkRevoked is set to true,
* verifies if the session corresponding to the session cookie was revoked. If the corresponding
* user's session was invalidated, an auth/session-cookie-revoked error is thrown. If not
* specified the check is not performed.
* the promise if the cookie could not be verified.
* If checkRevoked is set to true, first verifies whether the corresponding userinfo.disabled
* is true:
xil222 marked this conversation as resolved.
Show resolved Hide resolved
* If yes, an auth/user-disabled error is thrown.
* If no, verifies if the session corresponding to the session cookie was revoked.
* If the corresponding user's session was invalidated, an auth/session-cookie-revoked error
* is thrown.
* If not specified the check is not performed.
*
* @param {string} sessionCookie The session cookie to verify.
* @param {boolean=} checkRevoked Whether to check if the session cookie is revoked.
Expand All @@ -522,9 +529,9 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
const isEmulator = useEmulator();
return this.sessionCookieVerifier.verifyJWT(sessionCookie, isEmulator)
.then((decodedIdToken: DecodedIdToken) => {
// Whether to check if the token was revoked.
if (checkRevoked || isEmulator) {
return this.verifyDecodedJWTNotRevoked(
// Whether to check if the cookie was revoked.
if (checkRevoked || isEmulator) {
return this.verifyDecodedJWTNotRevokedOrDisabled(
decodedIdToken,
AuthClientErrorCode.SESSION_COOKIE_REVOKED);
}
Expand Down Expand Up @@ -723,20 +730,27 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> implements BaseAuthI
}

/**
* Verifies the decoded Firebase issued JWT is not revoked. Returns a promise that resolves
* with the decoded claims on success. Rejects the promise with revocation error if revoked.
* Verifies the decoded Firebase issued JWT is not revoked or disabled. Returns a promise that
* resolves with the decoded claims on success. Rejects the promise with revocation error if revoked
* or user disabled.
*
* @param {DecodedIdToken} decodedIdToken The JWT's decoded claims.
* @param {ErrorInfo} revocationErrorInfo The revocation error info to throw on revocation
* detection.
* @return {Promise<DecodedIdToken>} A Promise that will be fulfilled after a successful
* verification.
*/
private verifyDecodedJWTNotRevoked(
private verifyDecodedJWTNotRevokedOrDisabled(
decodedIdToken: DecodedIdToken, revocationErrorInfo: ErrorInfo): Promise<DecodedIdToken> {
// Get tokens valid after time for the corresponding user.
return this.getUser(decodedIdToken.sub)
.then((user: UserRecord) => {
// If user disabled, throw an auth/user-disabled error.
xil222 marked this conversation as resolved.
Show resolved Hide resolved
if (user.disabled) {
throw new FirebaseAuthError(
AuthClientErrorCode.USER_DISABLED,
'The user account has been disabled by an administrator');
xil222 marked this conversation as resolved.
Show resolved Hide resolved
}
// If no tokens valid after time available, token is not revoked.
if (user.tokensValidAfterTime) {
// Get the ID token authentication time and convert to milliseconds UTC.
Expand Down
4 changes: 4 additions & 0 deletions src/utils/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,10 @@ export class AuthClientErrorCode {
code: 'not-found',
message: 'The requested resource was not found.',
};
public static USER_DISABLED = {
code: 'user-disabled',
message: 'The user record is disabled.',
}
public static USER_NOT_DISABLED = {
code: 'user-not-disabled',
message: 'The user must be disabled in order to bulk delete it (or you must pass force=true).',
Expand Down
207 changes: 168 additions & 39 deletions test/integration/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,20 @@ describe('admin.auth', () => {
safeDelete(userRecord.uid);
}
});

it('A user with user record disabled is unable to sign in', async () => {
const password = 'password';
const email = '[email protected]';
return admin.auth().updateUser(updateUser.uid, { disabled : true , password, email })
.then(() => {
return clientAuth().signInWithEmailAndPassword(email, password);
})
.then(() => {
throw new Error('Unexpected success');
}, (error) => {
expect(error).to.have.property('code', 'auth/user-disabled');
});
});
});

it('getUser() fails when called with a non-existing UID', () => {
Expand Down Expand Up @@ -833,53 +847,120 @@ describe('admin.auth', () => {
});
});

it('verifyIdToken() fails when called with an invalid token', () => {
return admin.auth().verifyIdToken('invalid-token')
.should.eventually.be.rejected.and.have.property('code', 'auth/argument-error');
});
describe('verifyIdToken()', () => {
const uid = generateRandomString(20).toLowerCase();
const email = uid + '@example.com';
const password = 'password';
const userData = {
uid,
email,
emailVerified: false,
password,
};

// Create the test user before running this suite of tests.
before(() => {
return admin.auth().createUser(userData);
});

// Sign out after each test.
afterEach(() => {
return clientAuth().signOut();
});

after(() => {
return safeDelete(uid);
});

it('verifyIdToken() fails when called with an invalid token', () => {
return admin.auth().verifyIdToken('invalid-token')
.should.eventually.be.rejected.and.have.property('code', 'auth/argument-error');
});

if (authEmulatorHost) {
describe('Auth emulator support', () => {
const uid = 'authEmulatorUser';
before(() => {
return admin.auth().createUser({
uid,
email: '[email protected]',
password: 'p4ssword',
it('verifyIdToken() fails with checkRevoked set to true and corresponding user disabled', () => {
let currentIdToken: string;
let currentUser: User;
return clientAuth().signInWithEmailAndPassword(email, password)
.then(({ user }) => {
expect(user).to.exist;
expect(user!.email).to.equal(email);
currentUser = user!;
return user!.getIdToken();
})
.then((idToken) => {
currentIdToken = idToken;
return admin.auth().verifyIdToken(currentIdToken, true);
})
.then((decodedIdToken) => {
expect(decodedIdToken.uid).to.equal(uid);
expect(decodedIdToken.email).to.equal(email);
return admin.auth().updateUser(uid, { disabled: true });
})
.then((userRecord) => {
// Ensure disabled field has been updated.
expect(userRecord.uid).to.equal(uid);
expect(userRecord.email).to.equal(email);
expect(userRecord.disabled).to.equal(true);
return admin.auth().verifyIdToken(currentIdToken, false);
})
.then(() => {
xil222 marked this conversation as resolved.
Show resolved Hide resolved
// Verification should be successful.
// Need to reload user to ensure currentUser been updated.
return currentUser.reload();
})
.then(() => {
return admin.auth().verifyIdToken(currentIdToken, true);
})
.then(() => {
throw new Error('Unexpected success');
}, (error) => {
expect(error).to.have.property('code', 'auth/user-disabled');
});
});
after(() => {
return admin.auth().deleteUser(uid);
});
});

it('verifyIdToken() succeeds when called with an unsigned token', () => {
const unsignedToken = mocks.generateIdToken({
algorithm: 'none',
audience: projectId,
issuer: 'https://securetoken.google.com/' + projectId,
subject: uid,
if (authEmulatorHost) {
describe('Auth emulator support', () => {
const uid = 'authEmulatorUser';
before(() => {
return admin.auth().createUser({
uid,
email: '[email protected]',
password: 'p4ssword',
});
});
after(() => {
return admin.auth().deleteUser(uid);
});
return admin.auth().verifyIdToken(unsignedToken);
});

it('verifyIdToken() fails when called with a token with wrong project', () => {
const unsignedToken = mocks.generateIdToken({ algorithm: 'none', audience: 'nosuch' });
return admin.auth().verifyIdToken(unsignedToken)
.should.eventually.be.rejected.and.have.property('code', 'auth/argument-error');
});
it('verifyIdToken() succeeds when called with an unsigned token', () => {
const unsignedToken = mocks.generateIdToken({
algorithm: 'none',
audience: projectId,
issuer: 'https://securetoken.google.com/' + projectId,
subject: uid,
});
return admin.auth().verifyIdToken(unsignedToken);
});

it('verifyIdToken() fails when called with a token that does not belong to a user', () => {
const unsignedToken = mocks.generateIdToken({
algorithm: 'none',
audience: projectId,
issuer: 'https://securetoken.google.com/' + projectId,
subject: 'nosuch',
it('verifyIdToken() fails when called with a token with wrong project', () => {
const unsignedToken = mocks.generateIdToken({ algorithm: 'none', audience: 'nosuch' });
return admin.auth().verifyIdToken(unsignedToken)
.should.eventually.be.rejected.and.have.property('code', 'auth/argument-error');
});

it('verifyIdToken() fails when called with a token that does not belong to a user', () => {
const unsignedToken = mocks.generateIdToken({
algorithm: 'none',
audience: projectId,
issuer: 'https://securetoken.google.com/' + projectId,
subject: 'nosuch',
});
return admin.auth().verifyIdToken(unsignedToken)
.should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found');
});
return admin.auth().verifyIdToken(unsignedToken)
.should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found');
});
});
}
}
});

describe('Link operations', () => {
const uid = generateRandomString(20).toLowerCase();
Expand Down Expand Up @@ -1982,6 +2063,54 @@ describe('admin.auth', () => {
.should.eventually.be.rejected.and.have.property('code', 'auth/argument-error');
});
});

it('fails with checkRevoked set to true and corresponding user disabled', () => {
const expiresIn = 24 * 60 * 60 * 1000;
let currentIdToken: string;
let currentSessioncookie: string;
let currentUser: User;
return admin.auth().createCustomToken(uid, { admin: true, groupId: '1234' })
.then((customToken) => {
return clientAuth().signInWithCustomToken(customToken);
})
.then(({ user }) => {
expect(user).to.exist;
currentUser = user!;
return user!.getIdToken();
})
.then((idToken) => {
currentIdToken = idToken;
return admin.auth().verifyIdToken(idToken);
}).then((decodedIdTokenClaims) => {
expect(decodedIdTokenClaims.uid).to.be.equal(uid);
// One day long session cookie.
return admin.auth().createSessionCookie(currentIdToken, { expiresIn });
})
.then((sessionCookie) => {
currentSessioncookie = sessionCookie;
return admin.auth().verifySessionCookie(sessionCookie, true);
})
.then(() => {
return admin.auth().updateUser(uid, { disabled : true });
})
.then((userRecord) => {
// Ensure disabled field has been updated.
expect(userRecord.uid).to.equal(uid);
expect(userRecord.disabled).to.equal(true);
return admin.auth().verifySessionCookie(currentSessioncookie, false);
})
.then(() => {
// Need to reload user to ensure currentUser been updated.
return currentUser.reload();
}).then(() => {
admin.auth().verifySessionCookie(currentSessioncookie, true);
})
.then(() => {
throw new Error('Unexpected success');
}, (error) => {
expect(error).to.have.property('code', 'auth/user-disabled');
});
});
});

describe('importUsers()', () => {
Expand Down
38 changes: 38 additions & 0 deletions test/unit/auth/auth.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,25 @@ AUTH_CONFIGS.forEach((testConfig) => {
.should.eventually.be.rejectedWith('Decoding Firebase ID token failed');
});

it('should be rejected with checkRevoked set to true and corresponding user disabled', () => {
xil222 marked this conversation as resolved.
Show resolved Hide resolved
const expectedAccountInfoResponse = getValidGetAccountInfoResponse(tenantId);
expectedAccountInfoResponse.users[0].disabled = true;
const expectedUserRecordDisabled = getValidUserRecord(expectedAccountInfoResponse);
const getUserStub = sinon.stub(testConfig.Auth.prototype, 'getUser')
.resolves(expectedUserRecordDisabled);
expect(expectedUserRecordDisabled.disabled).to.be.equal(true);
stubs.push(getUserStub);
return auth.verifyIdToken(mockIdToken, true)
.then(() => {
throw new Error('Unexpected success');
}, (error) => {
// Confirm underlying API called with expected parameters.
expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
// Confirm expected error returned.
expect(error).to.have.property('code', 'auth/user-disabled');
});
});

it('should work with a non-cert credential when the GOOGLE_CLOUD_PROJECT environment variable is present', () => {
process.env.GOOGLE_CLOUD_PROJECT = mocks.projectId;

Expand Down Expand Up @@ -838,6 +857,25 @@ AUTH_CONFIGS.forEach((testConfig) => {
});
});

it('should be rejected with checkRevoked set to true and corresponding user disabled', () => {
xil222 marked this conversation as resolved.
Show resolved Hide resolved
const expectedAccountInfoResponse = getValidGetAccountInfoResponse(tenantId);
expectedAccountInfoResponse.users[0].disabled = true;
const expectedUserRecordDisabled = getValidUserRecord(expectedAccountInfoResponse);
const getUserStub = sinon.stub(testConfig.Auth.prototype, 'getUser')
.resolves(expectedUserRecordDisabled);
expect(expectedUserRecordDisabled.disabled).to.be.equal(true);
stubs.push(getUserStub);
return auth.verifyIdToken(mockSessionCookie, true)
xil222 marked this conversation as resolved.
Show resolved Hide resolved
.then(() => {
throw new Error('Unexpected success');
}, (error) => {
// Confirm underlying API called with expected parameters.
expect(getUserStub).to.have.been.calledOnce.and.calledWith(uid);
// Confirm expected error returned.
expect(error).to.have.property('code', 'auth/user-disabled');
});
});

it('should be fulfilled with checkRevoked set to true when no validSince available', () => {
// Simulate no validSince set on the user.
const noValidSinceGetAccountInfoResponse = getValidGetAccountInfoResponse(tenantId);
Expand Down