Skip to content

Commit 25f9389

Browse files
muralibasanigithub-actions[bot]
muralibasani
and
github-actions[bot]
authored
Sso issues (#2928)
* Changes for name, mail id in sso signup * Add tests * Add tests * Updating tests Signed-off-by: Muralidhar Basani <muralidhar.basani@aiven.io> * Fix with email claim * Update spec Signed-off-by: Muralidhar Basani <muralidhar.basani@aiven.io> * 🤖 Auto-update API types for TypeScript usage * Fix coral redirect * Fix logs --------- Signed-off-by: Muralidhar Basani <muralidhar.basani@aiven.io> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
1 parent 0294ab0 commit 25f9389

12 files changed

Lines changed: 191 additions & 66 deletions

File tree

‎coral/types/api.d.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7996,9 +7996,9 @@ export enum ApiPaths {
79967996
approveRequest = "/request/approve",
79977997
registerUser = "/registerUser",
79987998
promoteSchema = "/promote/schema",
7999-
deleteOperationalRequest = "/operationalRequest/reqId/:reqId/delete",
8000-
declineOperationalRequest = "/operationalRequest/reqId/:reqId/decline",
8001-
approveOperationalRequest = "/operationalRequest/reqId/:reqId/approve",
7999+
deleteOperationalRequest = "/operationalRequest/reqId/{reqId}/delete",
8000+
declineOperationalRequest = "/operationalRequest/reqId/{reqId}/decline",
8001+
approveOperationalRequest = "/operationalRequest/reqId/{reqId}/approve",
80028002
createConsumerOffsetsResetRequest = "/operationalRequest/consumerOffsetsReset/create",
80038003
logout = "/logout",
80048004
approveTopicRequests = "/execTopicRequests",
@@ -8031,24 +8031,24 @@ export enum ApiPaths {
80318031
createAcl = "/createAcl",
80328032
restartConnector = "/connector/restart",
80338033
changePwd = "/chPwd",
8034-
addEnvToCache = "/cache/tenant/:tenantId/entityType/environment",
8034+
addEnvToCache = "/cache/tenant/{tenantId}/entityType/environment",
80358035
addTenantId = "/addTenantId",
80368036
addRoleId = "/addRoleId",
80378037
addNewUser = "/addNewUser",
80388038
addNewTeam = "/addNewTeam",
80398039
addNewEnv = "/addNewEnv",
80408040
addNewCluster = "/addNewCluster",
8041-
claimAcl = "/acl/claim/:aclId",
8042-
getSwitchTeams = "/user/:userId/switchTeamsList",
8043-
getTopicRequest = "/topic/request/:topicReqId",
8041+
claimAcl = "/acl/claim/{aclId}",
8042+
getSwitchTeams = "/user/{userId}/switchTeamsList",
8043+
getTopicRequest = "/topic/request/{topicReqId}",
80448044
testClusterApiConnection = "/testClusterApiConnection",
80458045
shutdownApp = "/shutdownContext",
80468046
showUsers = "/showUserList",
8047-
getSchemaOfTopicFromSource = "/schemas/source/:source/kafkaEnv/:kafkaEnvId/topic/:topicName/schemaVersion/:schemaVersion",
8048-
getSchemaRequest = "/schema/request/:schemaReqId",
8047+
getSchemaOfTopicFromSource = "/schemas/source/{source}/kafkaEnv/{kafkaEnvId}/topic/{topicName}/schemaVersion/{schemaVersion}",
8048+
getSchemaRequest = "/schema/request/{schemaReqId}",
80498049
resetCache = "/resetCache",
80508050
getRequestStatistics = "/requests/statistics",
8051-
getOperationalRequests = "/operationalRequests/requestsFor/:requestsFor",
8051+
getOperationalRequests = "/operationalRequests/requestsFor/{requestsFor}",
80528052
validateOffsetRequestDetails = "/operationalRequest/consumerOffsetsReset/validate",
80538053
getRegistrationInfoFromId = "/getUserInfoFromRegistrationId",
80548054
getUserDetails = "/getUserDetails",
@@ -8126,11 +8126,11 @@ export enum ApiPaths {
81268126
getAclRequests = "/getAclRequests",
81278127
getAclRequestsForApprover = "/getAclRequestsForApprover",
81288128
getSchemaRegEnvsPaginated = "/environments/schemaRegistry",
8129-
getSchemaRegEnv = "/environments/schemaRegistry/:envId",
8129+
getSchemaRegEnv = "/environments/schemaRegistry/{envId}",
81308130
getKafkaConnectEnvsPaginated = "/environments/kafkaconnect",
8131-
getKafkaConnectEnv = "/environments/kafkaconnect/:envId",
8131+
getKafkaConnectEnv = "/environments/kafkaconnect/{envId}",
81328132
getKafkaEnvsPaginated = "/environments/kafka",
8133-
getKafkaEnv = "/environments/kafka/:envId",
8134-
getAclRequest = "/acl/request/:aclRequestId",
8135-
removeEnvFromCache = "/cache/tenant/:tenantId/entityType/environment/id/:id"
8133+
getKafkaEnv = "/environments/kafka/{envId}",
8134+
getAclRequest = "/acl/request/{aclRequestId}",
8135+
removeEnvFromCache = "/cache/tenant/{tenantId}/entityType/environment/id/{id}"
81368136
}

‎core/src/main/java/io/aiven/klaw/auth/KwAuthenticationSuccessHandler.java‎

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -62,25 +62,30 @@ public String getRedirectPage(HttpServletRequest request, Authentication authent
6262
String providerRoute = "{{ provider }}";
6363
String coralTopicsUri = "/coral/";
6464

65-
if (quickStartEnabled
66-
&& handleDbRequests
67-
.getUsersInfo(
68-
UtilMethods.getUserName(
69-
authentication.getPrincipal(), preferredUsernameAttribute, emailAttribute))
70-
.getRole()
71-
.equals(KwConstants.USER_ROLE)) {
72-
return coralTopicsUri;
73-
}
74-
75-
if (coralEnabled
76-
&& UtilControllerService.isCoralBuilt
77-
&& !handleDbRequests
78-
.getUsersInfo(
79-
UtilMethods.getUserName(
80-
authentication.getPrincipal(), preferredUsernameAttribute, emailAttribute))
81-
.getRole()
82-
.equals(KwConstants.SUPERADMIN_ROLE)) {
83-
return coralTopicsUri;
65+
try {
66+
if (quickStartEnabled
67+
&& handleDbRequests
68+
.getUsersInfo(
69+
UtilMethods.getUserName(
70+
authentication.getPrincipal(), preferredUsernameAttribute, emailAttribute))
71+
.getRole()
72+
.equals(KwConstants.USER_ROLE)) {
73+
return coralTopicsUri;
74+
}
75+
76+
if (coralEnabled
77+
&& UtilControllerService.isCoralBuilt
78+
&& !handleDbRequests
79+
.getUsersInfo(
80+
UtilMethods.getUserName(
81+
authentication.getPrincipal(), preferredUsernameAttribute, emailAttribute))
82+
.getRole()
83+
.equals(KwConstants.SUPERADMIN_ROLE)) {
84+
return coralTopicsUri;
85+
}
86+
} catch (Exception e) {
87+
log.info("Could not retrieve user info");
88+
return rootPath + indexPage;
8489
}
8590

8691
if (defaultSavedRequest == null) {

‎core/src/main/java/io/aiven/klaw/model/requests/ProfileModel.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public class ProfileModel implements Serializable {
1818
@Size(min = 5, max = 50, message = "Name must be atleast 5 characters")
1919
@Pattern(
2020
message = "Invalid Full name",
21-
regexp = "^[A-Za-zÀ-ÖØ-öø-ÿ' ]*$") // Pattern a-zA-z accents and umlaut and/or spaces.
21+
regexp = "^[A-Za-zÀ-ÖØ-öø-ÿ' ()]*$") // Pattern a-zA-z accents and umlaut and/or spaces.
2222
private String fullname;
2323

2424
@Email(message = "Email should be valid")

‎core/src/main/java/io/aiven/klaw/model/requests/RegisterUserInfoModel.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public class RegisterUserInfoModel implements Serializable {
3333
@NotNull(message = "Name cannot be null")
3434
@Pattern(
3535
message = "Invalid Full name.",
36-
regexp = "^[a-zA-z ]*$") // Pattern a-zA-z and/or spaces.
36+
regexp = "^[a-zA-Z ()]*$") // Pattern a-zA-z and/or spaces.
3737
private String fullname;
3838

3939
@Email(message = "Email should be valid")

‎core/src/main/java/io/aiven/klaw/service/UiControllerLoginService.java‎

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,9 @@ public class UiControllerLoginService {
4444
@Value("${klaw.login.authentication.type}")
4545
private String authenticationType;
4646

47+
@Value("${spring.ad.domain:#{null}}")
48+
private String adDomain;
49+
4750
@Value("${klaw.enable.authorization.ad:false}")
4851
private boolean enableUserAuthorizationFromAD;
4952

@@ -114,14 +117,19 @@ public String checkAnonymousLogin(
114117
String userName) {
115118
DefaultOAuth2User defaultOAuth2User = null;
116119
String nameAttribute = "name";
120+
String emailAttribute = "email";
121+
String emailAttributeVal = null;
117122
Collection<? extends GrantedAuthority> authorities = null;
118123

124+
Object principal = abstractAuthenticationToken.getPrincipal();
119125
// Extract attributes for user verification/registration
120126
if (abstractAuthenticationToken instanceof OAuth2AuthenticationToken) {
121-
defaultOAuth2User =
122-
(DefaultOAuth2User) SecurityContextHolder.getContext().getAuthentication().getPrincipal();
123-
nameAttribute = (String) defaultOAuth2User.getAttributes().get(nameAttribute);
124-
authorities = defaultOAuth2User.getAuthorities();
127+
if (principal instanceof DefaultOAuth2User) {
128+
defaultOAuth2User = (DefaultOAuth2User) principal;
129+
nameAttribute = (String) defaultOAuth2User.getAttributes().get(nameAttribute);
130+
emailAttributeVal = (String) defaultOAuth2User.getAttributes().get(emailAttribute);
131+
authorities = defaultOAuth2User.getAuthorities();
132+
}
125133
} else if (abstractAuthenticationToken instanceof UsernamePasswordAuthenticationToken) {
126134
nameAttribute = abstractAuthenticationToken.getName();
127135
authorities = abstractAuthenticationToken.getAuthorities();
@@ -146,7 +154,8 @@ public String checkAnonymousLogin(
146154
}
147155
}
148156
}
149-
return registerStagingUser(userName, nameAttribute, roleValidationPair.getRight());
157+
return registerStagingUser(
158+
userName, nameAttribute, roleValidationPair.getRight(), emailAttributeVal);
150159
}
151160

152161
if (abstractAuthenticationToken.isAuthenticated()) {
@@ -272,7 +281,8 @@ public String checkAuth(
272281
}
273282

274283
// register user with staging status, and forward to signup
275-
public String registerStagingUser(String userName, Object fullName, String roleFromAD) {
284+
public String registerStagingUser(
285+
String userName, Object fullName, String roleFromAD, String emailClaim) {
276286
try {
277287
log.info("User found in SSO/AD and not in Klaw db :{}", userName);
278288
String existingRegistrationId =
@@ -296,6 +306,11 @@ public String registerStagingUser(String userName, Object fullName, String roleF
296306
registerUserInfoModel.setRole(
297307
Objects.requireNonNullElse(roleFromAD, KwConstants.USER_ROLE));
298308
registerUserInfoModel.setRegisteredTime(new Timestamp(System.currentTimeMillis()));
309+
310+
if (emailClaim != null) {
311+
registerUserInfoModel.setMailid(emailClaim);
312+
}
313+
299314
registerUserInfoModel.setUsername(userName);
300315
registerUserInfoModel.setPwd("");
301316
if (fullName != null) {

‎core/src/main/java/io/aiven/klaw/service/UsersTeamsControllerService.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -508,7 +508,10 @@ public ApiResponse deleteUser(String userIdToDelete, boolean isExternal) throws
508508
}
509509

510510
try {
511-
inMemoryUserDetailsManager.deleteUser(userIdToDelete);
511+
if (inMemoryUserDetailsManager != null
512+
&& inMemoryUserDetailsManager.userExists(userIdToDelete)) {
513+
inMemoryUserDetailsManager.deleteUser(userIdToDelete);
514+
}
512515
String result = manageDatabase.getHandleDbRequests().deleteUserRequest(userIdToDelete);
513516
if (result.equals(ApiResultStatus.SUCCESS.value)) {
514517
commonUtilsService.updateMetadata(
@@ -628,8 +631,10 @@ public ApiResponse addNewUser(UserInfoModel newUser, boolean isExternal) throws
628631
: ApiResponse.notOk(result);
629632
} catch (Exception e) {
630633
try {
631-
if (inMemoryUserDetailsManager != null)
634+
if (inMemoryUserDetailsManager != null
635+
&& inMemoryUserDetailsManager.userExists(newUser.getUsername())) {
632636
inMemoryUserDetailsManager.deleteUser(newUser.getUsername());
637+
}
633638
} catch (Exception e1) {
634639
log.error("Try deleting user");
635640
}

‎core/src/main/resources/static/js/registerUsers.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -493,19 +493,19 @@ app.controller("registerUsersCtrl", function($scope, $http, $location, $window)
493493
return;
494494
}
495495

496-
if(!$scope.registerUser.emailid)
496+
if(!$scope.registerUser.mailid)
497497
{
498498
$scope.alertnote = "Email id is mandatory.";
499499
$scope.showAlertToast();
500500
return;
501501
}
502-
else if($scope.registerUser.emailid.length < 7)
502+
else if($scope.registerUser.mailid.length < 7)
503503
{
504504
$scope.alertnote = "Please enter a valid email id.";
505505
$scope.showAlertToast();
506506
return;
507507
}
508-
else if(!$scope.registerUser.emailid.includes("@"))
508+
else if(!$scope.registerUser.mailid.includes("@"))
509509
{
510510
$scope.alertnote = "Please enter a valid email id.";
511511
$scope.showAlertToast();
@@ -518,7 +518,7 @@ app.controller("registerUsersCtrl", function($scope, $http, $location, $window)
518518

519519
serviceInput['username'] = $scope.registerUser.username;
520520
serviceInput['fullname'] = $scope.registerUser.fullname;
521-
serviceInput['mailid'] = $scope.registerUser.emailid;
521+
serviceInput['mailid'] = $scope.registerUser.mailid;
522522
serviceInput['tenantName'] = tenantName.trim();
523523
serviceInput['pwd'] = '';
524524

‎core/src/main/resources/templates/registerLdap.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ <h3 class="p-2 rounded-title mb-3">Sign Up</h3>
354354
class="form-control"
355355
type="email"
356356
required
357-
ng-model="registerUser.emailid"
357+
ng-model="registerUser.mailid"
358358
/>
359359
</div>
360360
</div>

‎core/src/test/java/io/aiven/klaw/auth/KwAuthenticationSuccessHandlerTest.java‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,13 @@
11
package io.aiven.klaw.auth;
22

33
import static org.assertj.core.api.Assertions.assertThat;
4+
import static org.mockito.ArgumentMatchers.anyString;
5+
import static org.mockito.Mockito.doThrow;
6+
import static org.mockito.Mockito.mock;
47
import static org.mockito.Mockito.when;
58

9+
import io.aiven.klaw.helpers.db.rdbms.HandleDbRequestsJdbc;
10+
import io.aiven.klaw.service.UtilControllerService;
611
import jakarta.servlet.http.HttpServletRequest;
712
import jakarta.servlet.http.HttpSession;
813
import org.junit.jupiter.api.AfterEach;
@@ -12,8 +17,10 @@
1217
import org.junit.jupiter.api.TestMethodOrder;
1318
import org.junit.jupiter.api.extension.ExtendWith;
1419
import org.mockito.Mock;
20+
import org.springframework.security.core.Authentication;
1521
import org.springframework.security.web.savedrequest.DefaultSavedRequest;
1622
import org.springframework.test.context.junit.jupiter.SpringExtension;
23+
import org.springframework.test.util.ReflectionTestUtils;
1724

1825
@ExtendWith(SpringExtension.class)
1926
@TestMethodOrder(MethodOrderer.OrderAnnotation.class)
@@ -77,4 +84,30 @@ void getRedirectPageReturnFullPathRequestUriQueryTest() {
7784
kwAuthenticationSuccessHandler.getRedirectPage(httpServletRequest, null);
7885
assertThat(redirectedPage).isEqualTo("/browseTopics?topicName=testtopic");
7986
}
87+
88+
@Test
89+
void getRedirectPage_ReturnIndexWhenUserInfoThrowsException() {
90+
// Arrange
91+
when(httpServletRequest.getSession()).thenReturn(httpSession);
92+
when(httpSession.getAttribute(springSavedReqAttribute)).thenReturn(defaultSavedRequest);
93+
94+
Authentication mockAuthentication = mock(Authentication.class);
95+
HandleDbRequestsJdbc handleDbRequests = mock(HandleDbRequestsJdbc.class);
96+
97+
// Simulate exception when getUsersInfo is called
98+
when(mockAuthentication.getPrincipal()).thenReturn("userPrincipal");
99+
doThrow(new RuntimeException("DB error")).when(handleDbRequests).getUsersInfo(anyString());
100+
ReflectionTestUtils.setField(kwAuthenticationSuccessHandler, "coralEnabled", true);
101+
UtilControllerService.isCoralBuilt = true;
102+
103+
// Inject mocked handleDbRequests
104+
kwAuthenticationSuccessHandler.handleDbRequests = handleDbRequests;
105+
106+
// Act
107+
String redirectedPage =
108+
kwAuthenticationSuccessHandler.getRedirectPage(httpServletRequest, mockAuthentication);
109+
110+
// Assert
111+
assertThat(redirectedPage).isEqualTo("/index");
112+
}
80113
}

0 commit comments

Comments
 (0)