Skip to content

Commit 10225ed

Browse files
sudo87sudo87
committed
cleanup redundant LOGIN_SOURCE and limiting apis for first time login
1 parent 555b69b commit 10225ed

File tree

7 files changed

+41
-31
lines changed

7 files changed

+41
-31
lines changed

‎plugins/api/discovery/src/main/java/org/apache/cloudstack/discovery/ApiDiscoveryServiceImpl.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@
4444
import org.apache.cloudstack.api.response.ApiParameterResponse;
4545
import org.apache.cloudstack.api.response.ApiResponseResponse;
4646
import org.apache.cloudstack.api.response.ListResponse;
47+
import org.apache.cloudstack.resourcedetail.UserDetailVO;
4748
import org.apache.cloudstack.utils.reflectiontostringbuilderutils.ReflectionToStringBuilderUtils;
4849
import org.apache.commons.collections.CollectionUtils;
50+
import org.apache.commons.collections.MapUtils;
4951
import org.apache.commons.lang3.StringUtils;
5052
import org.reflections.ReflectionUtils;
5153
import org.springframework.stereotype.Component;
@@ -55,6 +57,7 @@
5557
import com.cloud.user.Account;
5658
import com.cloud.user.AccountService;
5759
import com.cloud.user.User;
60+
import com.cloud.user.UserAccount;
5861
import com.cloud.utils.ReflectUtil;
5962
import com.cloud.utils.component.ComponentLifecycleBase;
6063
import com.cloud.utils.component.PluggableService;
@@ -280,12 +283,23 @@ public ListResponse<? extends BaseResponse> listApis(User user, String name) {
280283
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid")));
281284
}
282285

283-
if (role.getRoleType() == RoleType.Admin && role.getId() == RoleType.Admin.getId()) {
284-
logger.info(String.format("Account [%s] is Root Admin, all APIs are allowed.",
285-
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid")));
286+
// Limit APIs on first login requiring password change
287+
UserAccount userAccount = accountService.getUserAccountById(user.getId());
288+
if (MapUtils.isNotEmpty(userAccount.getDetails()) &&
289+
userAccount.getDetails().containsKey(UserDetailVO.PasswordChangeRequired)) {
290+
291+
String needPasswordChange = userAccount.getDetails().get(UserDetailVO.PasswordChangeRequired);
292+
if ("true".equalsIgnoreCase(needPasswordChange)) {
293+
apisAllowed = Arrays.asList("login", "logout", "updateUser", "listUsers", "listApis");
294+
}
286295
} else {
287-
for (APIChecker apiChecker : _apiAccessCheckers) {
288-
apisAllowed = apiChecker.getApisAllowedToUser(role, user, apisAllowed);
296+
if (role.getRoleType() == RoleType.Admin && role.getId() == RoleType.Admin.getId()) {
297+
logger.info(String.format("Account [%s] is Root Admin, all APIs are allowed.",
298+
ReflectionToStringBuilderUtils.reflectOnlySelectedFields(account, "accountName", "uuid")));
299+
} else {
300+
for (APIChecker apiChecker : _apiAccessCheckers) {
301+
apisAllowed = apiChecker.getApisAllowedToUser(role, user, apisAllowed);
302+
}
289303
}
290304
}
291305

‎plugins/api/discovery/src/test/java/org/apache/cloudstack/discovery/ApiDiscoveryTest.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.cloud.user.AccountService;
2222
import com.cloud.user.AccountVO;
2323
import com.cloud.user.User;
24+
import com.cloud.user.UserAccount;
2425
import com.cloud.user.UserVO;
2526

2627
import org.apache.cloudstack.acl.APIChecker;
@@ -44,6 +45,7 @@
4445

4546
import static org.mockito.ArgumentMatchers.any;
4647
import static org.mockito.ArgumentMatchers.anyList;
48+
import static org.mockito.ArgumentMatchers.anyLong;
4749

4850
@RunWith(MockitoJUnitRunner.class)
4951
public class ApiDiscoveryTest {
@@ -66,12 +68,17 @@ public class ApiDiscoveryTest {
6668
@InjectMocks
6769
ApiDiscoveryServiceImpl discoveryServiceSpy;
6870

71+
@Mock
72+
UserAccount mockUserAccount;
73+
6974
@Before
7075
public void setup() {
7176
discoveryServiceSpy.s_apiNameDiscoveryResponseMap = apiNameDiscoveryResponseMapMock;
7277
discoveryServiceSpy._apiAccessCheckers = apiAccessCheckersMock;
7378

7479
Mockito.when(discoveryServiceSpy._apiAccessCheckers.iterator()).thenReturn(Arrays.asList(apiCheckerMock).iterator());
80+
Mockito.when(mockUserAccount.getDetails()).thenReturn(null);
81+
Mockito.when(accountServiceMock.getUserAccountById(anyLong())).thenReturn(mockUserAccount);
7582
}
7683

7784
private User getTestUser() {

‎ui/public/locales/en.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -527,6 +527,7 @@
527527
"label.change.ipaddress": "Change IP address for NIC",
528528
"label.change.disk.offering": "Change disk offering",
529529
"label.change.offering.for.volume": "Change disk offering for the volume",
530+
"label.change.password.onlogin": "User must change password at next login",
530531
"label.change.service.offering": "Change service offering",
531532
"label.character": "Character",
532533
"label.checksum": "Checksum",
@@ -1096,7 +1097,6 @@
10961097
"label.forced": "Force",
10971098
"label.force.convert.to.pool": "Force converting to storage pool directly (not using temporary storage for conversion)",
10981099
"label.force.ms.to.import.vm.files": "Enable to force OVF Download via Management Server. Disable to use KVM Host ovftool (if installed)",
1099-
"label.force.password.reset": "Force password change on next login",
11001100
"label.force.update.os.type": "Force update OS type",
11011101
"label.force.stop": "Force stop",
11021102
"label.force.reboot": "Force reboot",

‎ui/src/permission.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,11 @@ router.beforeEach((to, from, next) => {
9494
}
9595
store.commit('SET_LOGIN_FLAG', true)
9696
}
97+
// store already loaded
9798
if (store.getters.passwordChangeRequired) {
98-
// Only allow the Change Password page
9999
if (to.path === '/user/forceChangePassword') {
100100
next()
101101
} else {
102-
// Redirect everything else (including dashboard, wildcards) to Change Password
103102
next({ path: '/user/forceChangePassword' })
104103
NProgress.done()
105104
}
@@ -113,6 +112,7 @@ router.beforeEach((to, from, next) => {
113112
store
114113
.dispatch('GetInfo')
115114
.then(apis => {
115+
// Essential for Page Refresh scenarios
116116
if (store.getters.passwordChangeRequired) {
117117
// Only allow the Change Password page
118118
if (to.path === '/user/forceChangePassword') {

‎ui/src/store/modules/user.js‎

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,7 @@ import {
4545
OAUTH_DOMAIN,
4646
OAUTH_PROVIDER,
4747
LATEST_CS_VERSION,
48-
PASSWORD_CHANGE_REQUIRED,
49-
LOGIN_SOURCE
48+
PASSWORD_CHANGE_REQUIRED
5049
} from '@/store/mutation-types'
5150

5251
import {
@@ -83,8 +82,7 @@ const user = {
8382
twoFaIssuer: '',
8483
customHypervisorName: 'Custom',
8584
readyForShutdownPollingJob: '',
86-
passwordChangeRequired: false,
87-
loginSource: ''
85+
passwordChangeRequired: false
8886
},
8987

9088
mutations: {
@@ -204,13 +202,10 @@ const user = {
204202
SET_PASSWORD_CHANGE_REQUIRED: (state, required) => {
205203
state.passwordChangeRequired = required
206204
if (required) {
207-
vueProps.$localStorage.set('PASSWORD_CHANGE_REQUIRED', 'true')
205+
vueProps.$localStorage.set(PASSWORD_CHANGE_REQUIRED, true)
208206
} else {
209-
vueProps.$localStorage.remove('PASSWORD_CHANGE_REQUIRED')
207+
vueProps.$localStorage.remove(PASSWORD_CHANGE_REQUIRED)
210208
}
211-
},
212-
SET_LOGIN_SOURCE: (state, source) => {
213-
vueProps.$localStorage.set('LOGIN_SOURCE', source)
214209
}
215210
},
216211

@@ -259,8 +254,7 @@ const user = {
259254
if (result && result.managementserverid) {
260255
commit('SET_MS_ID', result.managementserverid)
261256
}
262-
commit('SET_LOGIN_SOURCE', 'password')
263-
if (result.passwordchangerequired && result.passwordchangerequired === 'true') {
257+
if (result.passwordchangerequired) {
264258
commit('SET_PASSWORD_CHANGE_REQUIRED', true)
265259
commit('SET_APIS', {})
266260
vueProps.$localStorage.remove(APIS)
@@ -321,8 +315,6 @@ const user = {
321315
const latestVersion = vueProps.$localStorage.get(LATEST_CS_VERSION, { version: '', fetchedTs: 0 })
322316
commit('SET_LATEST_VERSION', latestVersion)
323317
notification.destroy()
324-
commit('SET_LOGIN_SOURCE', 'oauth')
325-
commit('SET_PASSWORD_CHANGE_REQUIRED', false)
326318
resolve()
327319
}).catch(error => {
328320
reject(error)
@@ -349,11 +341,9 @@ const user = {
349341
commit('SET_LATEST_VERSION', latestVersion)
350342

351343
// This block is to enforce password change for first time login after admin resets password
352-
const loginSource = vueProps.$localStorage.get(LOGIN_SOURCE)
353-
const isPwdChangeRequired = vueProps.$localStorage.get(PASSWORD_CHANGE_REQUIRED) === 'true'
354-
const isPwdChangeRequiredForLogin = (loginSource === 'password' && isPwdChangeRequired)
355-
commit('SET_PASSWORD_CHANGE_REQUIRED', isPwdChangeRequiredForLogin)
356-
if (isPwdChangeRequiredForLogin) {
344+
const isPwdChangeRequired = vueProps.$localStorage.get(PASSWORD_CHANGE_REQUIRED)
345+
commit('SET_PASSWORD_CHANGE_REQUIRED', isPwdChangeRequired)
346+
if (isPwdChangeRequired) {
357347
getAPI('listUsers', { id: Cookies.get('userid') }).then(response => {
358348
const result = response.listusersresponse.user[0]
359349
commit('SET_INFO', result)
@@ -529,9 +519,7 @@ const user = {
529519
vueProps.$localStorage.remove(HEADER_NOTICES)
530520

531521
vueProps.$localStorage.remove(PASSWORD_CHANGE_REQUIRED)
532-
vueProps.$localStorage.remove(LOGIN_SOURCE)
533522
commit('SET_PASSWORD_CHANGE_REQUIRED', false)
534-
commit('SET_LOGIN_SOURCE', '')
535523

536524
logout(state.token).then(() => {
537525
message.destroy()

‎ui/src/store/mutation-types.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,6 @@ export const MS_ID = 'MS_ID'
4444
export const OAUTH_DOMAIN = 'OAUTH_DOMAIN'
4545
export const OAUTH_PROVIDER = 'OAUTH_PROVIDER'
4646
export const PASSWORD_CHANGE_REQUIRED = 'PASSWORD_CHANGE_REQUIRED'
47-
export const LOGIN_SOURCE = 'LOGIN_SOURCE'
4847

4948
export const CONTENT_WIDTH_TYPE = {
5049
Fluid: 'Fluid',

‎ui/src/views/iam/ChangeUserPassword.vue‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,11 @@
5050
:placeholder="$t('label.confirmpassword.description')"/>
5151
</a-form-item>
5252
<a-form-item v-if="isAdminOrDomainAdmin() && isNormalUserResource()" name="passwordChangeRequired" ref="passwordChangeRequired">
53-
<a-switch v-model:checked="form.passwordChangeRequired" />
54-
<span style="padding-left: 10px;">{{ $t('label.force.password.reset') }}</span>
53+
<a-checkbox v-model:checked="form.passwordChangeRequired">
54+
{{ $t('label.change.password.onlogin') }}
55+
</a-checkbox>
5556
</a-form-item>
57+
5658
<div :span="24" class="action-button">
5759
<a-button @click="closeAction">{{ $t('label.cancel') }}</a-button>
5860
<a-button :loading="loading" ref="submit" type="primary" @click="handleSubmit">{{ $t('label.ok') }}</a-button>

0 commit comments

Comments
 (0)