fix(ui): pick email and name based on claim values at time of login (#17626)

* fix(ui): pick email and name based on claim values at time of login

* Update Error Message

* Update code for better message

* Typo

* fix playwright failures

* fix playwright tests

* fix tests

* fix login spec failing

---------

Co-authored-by: mohitdeuex <mohit.y@deuexsolutions.com>
(cherry picked from commit fa3529f0850ed801e11b752b963ab0cafb637e87)
This commit is contained in:
Chirag Madlani 2024-08-30 11:57:37 +05:30 committed by Chira Madlani
parent 29a29cde4d
commit 2a3ba61703
9 changed files with 206 additions and 18 deletions

View File

@ -24,7 +24,15 @@ public final class BadRequestException extends WebServiceException {
super(Response.Status.BAD_REQUEST, ERROR_TYPE, DEFAULT_MESSAGE); super(Response.Status.BAD_REQUEST, ERROR_TYPE, DEFAULT_MESSAGE);
} }
private BadRequestException(String message) {
super(Response.Status.BAD_REQUEST, ERROR_TYPE, message);
}
public static BadRequestException of() { public static BadRequestException of() {
return new BadRequestException(); return new BadRequestException();
} }
public static BadRequestException of(String message) {
return new BadRequestException(message);
}
} }

View File

@ -3747,6 +3747,13 @@ public interface CollectionDAO {
@SqlQuery("SELECT COUNT(*) FROM user_entity WHERE LOWER(email) = LOWER(:email)") @SqlQuery("SELECT COUNT(*) FROM user_entity WHERE LOWER(email) = LOWER(:email)")
int checkEmailExists(@Bind("email") String email); int checkEmailExists(@Bind("email") String email);
@SqlQuery("SELECT COUNT(*) FROM user_entity WHERE LOWER(name) = LOWER(:name)")
int checkUserNameExists(@Bind("name") String name);
@SqlQuery(
"SELECT json FROM user_entity WHERE LOWER(name) = LOWER(:name) AND LOWER(email) = LOWER(:email)")
String findUserByNameAndEmail(@Bind("name") String name, @Bind("email") String email);
@SqlQuery("SELECT json FROM user_entity WHERE LOWER(email) = LOWER(:email)") @SqlQuery("SELECT json FROM user_entity WHERE LOWER(email) = LOWER(:email)")
String findUserByEmail(@Bind("email") String email); String findUserByEmail(@Bind("email") String email);

View File

@ -60,6 +60,7 @@ import org.openmetadata.schema.type.csv.CsvImportResult;
import org.openmetadata.schema.utils.EntityInterfaceUtil; import org.openmetadata.schema.utils.EntityInterfaceUtil;
import org.openmetadata.service.Entity; import org.openmetadata.service.Entity;
import org.openmetadata.service.OpenMetadataApplicationConfig; import org.openmetadata.service.OpenMetadataApplicationConfig;
import org.openmetadata.service.exception.BadRequestException;
import org.openmetadata.service.exception.CatalogExceptionMessage; import org.openmetadata.service.exception.CatalogExceptionMessage;
import org.openmetadata.service.exception.EntityNotFoundException; import org.openmetadata.service.exception.EntityNotFoundException;
import org.openmetadata.service.jdbi3.CollectionDAO.EntityRelationshipRecord; import org.openmetadata.service.jdbi3.CollectionDAO.EntityRelationshipRecord;
@ -146,7 +147,21 @@ public class UserRepository extends EntityRepository<User> {
} }
public User getByEmail(UriInfo uriInfo, String email, Fields fields) { public User getByEmail(UriInfo uriInfo, String email, Fields fields) {
String userString = ((CollectionDAO.UserDAO) dao).findUserByEmail(email); String userString = daoCollection.userDAO().findUserByEmail(email);
if (userString == null) {
throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entityNotFound(USER, email));
}
User user = JsonUtils.readValue(userString, User.class);
setFieldsInternal(user, fields);
setInheritedFields(user, fields);
// Clone the entity
User entityClone = JsonUtils.deepCopy(user, User.class);
clearFieldsInternal(entityClone, fields);
return withHref(uriInfo, entityClone);
}
public User getUserByNameAndEmail(UriInfo uriInfo, String name, String email, Fields fields) {
String userString = daoCollection.userDAO().findUserByNameAndEmail(name, email);
if (userString == null) { if (userString == null) {
throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entityNotFound(USER, email)); throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entityNotFound(USER, email));
} }
@ -314,6 +329,10 @@ public class UserRepository extends EntityRepository<User> {
return daoCollection.userDAO().checkEmailExists(emailId) > 0; return daoCollection.userDAO().checkEmailExists(emailId) > 0;
} }
public boolean checkUserNameExists(String username) {
return daoCollection.userDAO().checkUserNameExists(username) > 0;
}
public void initializeUsers(OpenMetadataApplicationConfig config) { public void initializeUsers(OpenMetadataApplicationConfig config) {
AuthProvider authProvider = config.getAuthenticationConfiguration().getProvider(); AuthProvider authProvider = config.getAuthenticationConfiguration().getProvider();
// Create Admins // Create Admins
@ -360,19 +379,45 @@ public class UserRepository extends EntityRepository<User> {
public List<EntityReference> getGroupTeams( public List<EntityReference> getGroupTeams(
UriInfo uriInfo, SecurityContext context, String email) { UriInfo uriInfo, SecurityContext context, String email) {
// Cleanup // Cleanup
User user = getByEmail(uriInfo, email, Fields.EMPTY_FIELDS); User user =
validateLoggedInUserNameAndEmailMatches(context.getUserPrincipal().getName(), email, user); getLoggedInUserByNameAndEmail(
uriInfo, context.getUserPrincipal().getName(), email, Fields.EMPTY_FIELDS);
List<EntityReference> teams = getTeams(user); List<EntityReference> teams = getTeams(user);
return getGroupTeams(teams); return getGroupTeams(teams);
} }
public void validateLoggedInUserNameAndEmailMatches( public User getLoggedInUserByNameAndEmail(
String username, String email, User storedUser) { UriInfo uriInfo, String username, String email, Fields fields) {
String lowerCasedName = username.toLowerCase(); try {
String lowerCasedEmail = email.toLowerCase(); return getUserByNameAndEmail(uriInfo, username, email, fields);
if (!(lowerCasedName.equals(storedUser.getName().toLowerCase()) } catch (EntityNotFoundException e) {
&& lowerCasedEmail.equals(storedUser.getEmail().toLowerCase()))) { boolean existByName = checkUserNameExists(username);
throw EntityNotFoundException.byMessage(CatalogExceptionMessage.entityNotFound(USER, email)); boolean existByEmail = checkEmailAlreadyExists(email);
if (existByName && !existByEmail) {
User userByName = getByName(uriInfo, username, Fields.EMPTY_FIELDS);
throw BadRequestException.of(
String.format(
"User with given name exists but is not associated with the provided email. "
+ "Matching User Found By Name [username:email] : [%s:%s], Provided User: [%s:%s]",
userByName.getName().toLowerCase(),
userByName.getEmail().toLowerCase(),
username,
email));
} else if (!existByName && existByEmail) {
User userByEmail = getByEmail(uriInfo, email, Fields.EMPTY_FIELDS);
throw BadRequestException.of(
String.format(
"User with given email exists but is not associated with provider username. "
+ "Matching User Found By Email [username:email] : [%s:%s], Provided User: [%s:%s]",
userByEmail.getName().toLowerCase(),
userByEmail.getEmail().toLowerCase(),
username,
email));
} else {
throw EntityNotFoundException.byMessage(
String.format(
"User with provider name : %s and email : %s not found", username, email));
}
} }
} }

View File

@ -450,10 +450,9 @@ public class UserResource extends EntityResource<User, UserRepository> {
(CatalogSecurityContext) containerRequestContext.getSecurityContext(); (CatalogSecurityContext) containerRequestContext.getSecurityContext();
Fields fields = getFields(fieldsParam); Fields fields = getFields(fieldsParam);
String currentEmail = ((CatalogPrincipal) catalogSecurityContext.getUserPrincipal()).getEmail(); String currentEmail = ((CatalogPrincipal) catalogSecurityContext.getUserPrincipal()).getEmail();
User user = repository.getByEmail(uriInfo, currentEmail, fields); User user =
repository.getLoggedInUserByNameAndEmail(
repository.validateLoggedInUserNameAndEmailMatches( uriInfo, catalogSecurityContext.getUserPrincipal().getName(), currentEmail, fields);
securityContext.getUserPrincipal().getName(), currentEmail, user);
// Sync the Roles from token to User // Sync the Roles from token to User
if (Boolean.TRUE.equals(authorizerConfiguration.getUseRolesFromProvider()) if (Boolean.TRUE.equals(authorizerConfiguration.getUseRolesFromProvider())

View File

@ -66,7 +66,9 @@ test.describe('Glossary tests', () => {
const glossary1 = new Glossary(); const glossary1 = new Glossary();
glossary1.data.owners = [{ name: 'admin', type: 'user' }]; glossary1.data.owners = [{ name: 'admin', type: 'user' }];
glossary1.data.mutuallyExclusive = true; glossary1.data.mutuallyExclusive = true;
glossary1.data.reviewers = [{ name: user1.getUserName(), type: 'user' }]; glossary1.data.reviewers = [
{ name: `${user1.data.firstName}${user1.data.lastName}`, type: 'user' },
];
glossary1.data.terms = [new GlossaryTerm(glossary1)]; glossary1.data.terms = [new GlossaryTerm(glossary1)];
await test.step('Create Glossary', async () => { await test.step('Create Glossary', async () => {

View File

@ -67,6 +67,7 @@ import {
getUrlPathnameExpiry, getUrlPathnameExpiry,
getUserManagerConfig, getUserManagerConfig,
isProtectedRoute, isProtectedRoute,
prepareUserProfileFromClaims,
} from '../../../utils/AuthProvider.util'; } from '../../../utils/AuthProvider.util';
import { escapeESReservedCharacters } from '../../../utils/StringsUtils'; import { escapeESReservedCharacters } from '../../../utils/StringsUtils';
import { showErrorToast, showInfoToast } from '../../../utils/ToastUtils'; import { showErrorToast, showInfoToast } from '../../../utils/ToastUtils';
@ -122,6 +123,9 @@ export const AuthProvider = ({
setAuthConfig, setAuthConfig,
setAuthorizerConfig, setAuthorizerConfig,
setIsSigningUp, setIsSigningUp,
authorizerConfig,
jwtPrincipalClaims,
jwtPrincipalClaimsMapping,
setJwtPrincipalClaims, setJwtPrincipalClaims,
setJwtPrincipalClaimsMapping, setJwtPrincipalClaimsMapping,
removeRefreshToken, removeRefreshToken,
@ -376,10 +380,18 @@ export const AuthProvider = ({
? userAPIQueryFields + ',' + isEmailVerifyField ? userAPIQueryFields + ',' + isEmailVerifyField
: userAPIQueryFields; : userAPIQueryFields;
try { try {
const newUser = prepareUserProfileFromClaims({
user,
jwtPrincipalClaims,
principalDomain: authorizerConfig?.principalDomain ?? '',
jwtPrincipalClaimsMapping,
clientType,
});
const res = await getLoggedInUser({ fields }); const res = await getLoggedInUser({ fields });
if (res) { if (res) {
const updatedUserData = getUserDataFromOidc(res, user); const updatedUserData = getUserDataFromOidc(res, newUser);
if (!matchUserDetails(res, updatedUserData, ['email'])) { if (!matchUserDetails(res, updatedUserData, ['profile', 'email'])) {
getUpdatedUser(updatedUserData, res); getUpdatedUser(updatedUserData, res);
} else { } else {
setCurrentUser(res); setCurrentUser(res);
@ -414,6 +426,10 @@ export const AuthProvider = ({
}, },
[ [
authConfig?.enableSelfSignup, authConfig?.enableSelfSignup,
clientType,
authorizerConfig?.principalDomain,
jwtPrincipalClaims,
jwtPrincipalClaimsMapping,
setIsSigningUp, setIsSigningUp,
setIsAuthenticated, setIsAuthenticated,
setApplicationLoading, setApplicationLoading,

View File

@ -39,6 +39,7 @@ import {
} from '../../../utils/ToastUtils'; } from '../../../utils/ToastUtils';
import { resetWebAnalyticSession } from '../../../utils/WebAnalyticsUtils'; import { resetWebAnalyticSession } from '../../../utils/WebAnalyticsUtils';
import { toLower } from 'lodash';
import { useApplicationStore } from '../../../hooks/useApplicationStore'; import { useApplicationStore } from '../../../hooks/useApplicationStore';
import { OidcUser } from './AuthProvider.interface'; import { OidcUser } from './AuthProvider.interface';
@ -115,7 +116,7 @@ const BasicAuthProvider = ({
onLoginSuccess({ onLoginSuccess({
id_token: response.accessToken, id_token: response.accessToken,
profile: { profile: {
email: response.email, email: toLower(email),
name: '', name: '',
picture: '', picture: '',
sub: '', sub: '',

View File

@ -22,6 +22,7 @@ import { first, get, isEmpty, isNil } from 'lodash';
import { WebStorageStateStore } from 'oidc-client'; import { WebStorageStateStore } from 'oidc-client';
import { import {
AuthenticationConfigurationWithScope, AuthenticationConfigurationWithScope,
OidcUser,
UserProfile, UserProfile,
} from '../components/Auth/AuthProviders/AuthProvider.interface'; } from '../components/Auth/AuthProviders/AuthProvider.interface';
import { REDIRECT_PATHNAME, ROUTES } from '../constants/constants'; import { REDIRECT_PATHNAME, ROUTES } from '../constants/constants';
@ -377,3 +378,46 @@ export const setUrlPathnameExpiryAfterRoute = (pathname: string) => {
path: '/', path: '/',
}); });
}; };
/**
* We support Principle claim as: email,preferred_username,sub in any order
* When Users are created from the initialAdmin we want to pick correct user details based on the principle claim
* This method will ensure that name & email are correctly picked from the principle claim
* @param user - User details extracted from Token
* @param jwtPrincipalClaims - List of principle claims coming from auth API response
* @param principalDomain - Principle Domain value coming from
* @param jwtPrincipalClaimsMapping - Mapping of principle claims to user profile
* @param clientType - Client Type Public or Confidential
* @returns OidcUser with Profile info plucked based on the principle claim
*/
export const prepareUserProfileFromClaims = ({
user,
jwtPrincipalClaims,
principalDomain,
jwtPrincipalClaimsMapping,
clientType,
}: {
user: OidcUser;
jwtPrincipalClaims: string[];
principalDomain: string;
jwtPrincipalClaimsMapping: string[];
clientType: ClientType;
}): OidcUser => {
const newUser = {
...user,
profile:
clientType === ClientType.Public
? getNameFromUserData(
user.profile,
jwtPrincipalClaims,
principalDomain,
jwtPrincipalClaimsMapping
)
: {
name: user.profile?.name ?? '',
email: user.profile?.email ?? '',
},
} as OidcUser;
return newUser;
};

View File

@ -100,3 +100,69 @@ describe('Test Auth Provider utils', () => {
expect(email).toEqual('i_am_preferred_username@test.com'); expect(email).toEqual('i_am_preferred_username@test.com');
}); });
}); });
import { OidcUser } from '../components/Auth/AuthProviders/AuthProvider.interface';
import { ClientType } from '../generated/configuration/authenticationConfiguration';
import { prepareUserProfileFromClaims } from './AuthProvider.util';
describe('prepareUserProfileFromClaims', () => {
const mockUser: OidcUser = {
profile: {
name: 'John Doe',
email: 'john.doe@example.com',
},
} as OidcUser;
const mockJwtPrincipalClaims = ['email'];
const mockPrincipalDomain = 'example.com';
const mockJwtPrincipalClaimsMapping = ['username:name', 'email:email'];
it('should prepare user profile for public client type', () => {
const result = prepareUserProfileFromClaims({
user: mockUser,
jwtPrincipalClaims: mockJwtPrincipalClaims,
principalDomain: mockPrincipalDomain,
jwtPrincipalClaimsMapping: mockJwtPrincipalClaimsMapping,
clientType: ClientType.Public,
});
expect(result.profile).toEqual({
name: 'John Doe',
email: 'john.doe@example.com',
});
});
it('should prepare user profile for non-public client type', () => {
const result = prepareUserProfileFromClaims({
user: mockUser,
jwtPrincipalClaims: mockJwtPrincipalClaims,
principalDomain: mockPrincipalDomain,
jwtPrincipalClaimsMapping: mockJwtPrincipalClaimsMapping,
clientType: ClientType.Confidential,
});
expect(result.profile).toEqual({
name: 'John Doe',
email: 'john.doe@example.com',
});
});
it('should handle missing profile fields for non-public client type', () => {
const mockUserWithMissingFields: OidcUser = {
profile: {},
} as OidcUser;
const result = prepareUserProfileFromClaims({
user: mockUserWithMissingFields,
jwtPrincipalClaims: mockJwtPrincipalClaims,
principalDomain: mockPrincipalDomain,
jwtPrincipalClaimsMapping: mockJwtPrincipalClaimsMapping,
clientType: ClientType.Confidential,
});
expect(result.profile).toEqual({
name: '',
email: '',
});
});
});