Repository navigation
fix(auth): validate tenantId and providerId against URL path traversal - #3292
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces stricter validation for tenant IDs and provider IDs (SAML and OIDC) across the authentication module, replacing basic non-empty string checks with regex-based validation and adding a new isTenantId utility. While these changes improve input validation, the review feedback identifies a security vulnerability in the regular expressions used in auth-config.ts and validator.ts. Specifically, the unescaped hyphen in the character class [a-zA-Z0-9-_.~] is interpreted as a range (9-_), which inadvertently permits dangerous characters like ?, @, and \. Escaping the hyphens is highly recommended to prevent potential validation bypasses.
lahirumaramba
left a comment
There was a problem hiding this comment.
LGTM! Added a nit comment. Thanks!
Description
Validates
tenantIdand OIDC/SAMLproviderIdresource identifiers against URL path traversal, dot-segments (.,..,..., trailing.), query/fragment injection (?,#,&), and percent-encoded sequences (%) before interpolating them into Identity Toolkit REST API URLs:validator.isTenantId(tenantId)enforcing that tenant IDs are non-empty strings consisting of RFC 3986 unreserved characters ([a-zA-Z0-9-_.~]) that do not end with..validator.isTenantId(tenantId)inTenantManager.authForTenant,TenantAwareAuth,AuthRequestHandler.getTenant,AuthRequestHandler.deleteTenant,AuthRequestHandler.updateTenant,TenantAwareAuthRequestHandler, andvalidateCreateEditRequest.SAMLConfig.isProviderId,SAMLConfig.validate,OIDCConfig.isProviderId, andOIDCConfig.validateto require a non-empty suffix aftersaml./oidc.consisting of[a-zA-Z0-9-_.~]and not ending with..validator,TenantManager,TenantAwareAuth,SAMLConfig,OIDCConfig, andAuthRequestHandler.