Unable to login with a newly created user about manageiq-api

[imtayadeway]

@mzazrivec I know that conversations about this issue have occurred variously on gitter etc.. - can you provide a few details here to help consolidate what we know;

1. What's the expected behavior?
2. Was there a time when the API performed the expected behavior, and
3. If so, can you identify a revision/release when the product did the right thing?

If you can also clarify the relationships between your user/role/groups in setup steps above?

I don't think I have a complete picture of what is going on - I think it was speculated on gitter that the reason it fails is because the user doesn't yet have a current group. I'm convinced this isn't the issue - we have had tests around this case which haven't changed in a long time - certainly predating the change in #359 .

Thanks!

[himdel]

[WIP answer, still looking for some details... (the TODOs are for myself)]

@imtayadeway @mzazrivec is on PTO, I'll try to provide the details...

1. What's the expected behavior?

Ideally this should behave identically to the Rails login (EDIT: turns out it does!)

Practically, the login just needs to not fail and needs to pick any group from user.miq_groups to be the current_group.

(In theory we could fail with a list of possible groups instead of picking one, but that potentially leaks info about user's group membership and would require extra UI.)

2. Was there a time when the API performed the expected behavior, and

Before ManageIQ/manageiq-ui-classic#1752 this was a non-issue for non-LDAP users - only those could be a member of multiple groups. (TODO find out if it worked for LDAP - but it does seem to involve current_group.try)

Before #359, the exception did not fail the API login.

(Theoretically, it is possible that this would have worked even if the API doesn't really reply with a token, assuming the response is a 200. At least before ManageIQ/manageiq-ui-classic#2715, as being logged in the API was not strictly a requirement then.)

3. If so, can you identify a revision/release when the product did the right thing?

Before #359, the exception did not fail the API login.

The API never used to assign to current_group as far as I'm aware.

If you can also clarify the relationships between your user/role/groups in setup steps above?

Create a new user in the UI, assign a group, save. The group will be in user.miq_groups but user.current_group will be nil.

Since users can have multiple groups now, it does not get picked for current_group.

BTW this also means we can work around the issue by always setting current_group when editing the users in the UI, but this won't affect already created users, and I assume user creation via the API would be equally affected unless you explicitly call the set_current_group action.

... Except the API currently does not support creating users with multiple groups, because while parse_set_group can handle reading from miq_groups, it won't do so if group exists, and validate_user_create_data fails without group :). But using group always sets current_group, so API user creation is not affected.

EDIT: This really depends on how the user gets created... the problem is this..

u = (user with everything right, except for miq_groups and current_group)
u.save! # because we can't change groups before validating
u.miq_groups = [...]  # changes users_groups immediately, but only does self.current_group=... without self.save
# u.save because we already did

leads to current_group being nil in the DB (only set in the instance).

I don't think I have a complete picture of what is going on - I think it was speculated on gitter that the reason it fails is because the user doesn't yet have a current group. I'm convinced this isn't the issue - we have had tests around this case which haven't changed in a long time - certainly predating the change in #359

Can you point me to those tests please?

But .. making doubly sure then...

1. created a user in the UI, give it just one group, save, and check in the console:

> u = User.last ; [ u.userid, u.current_group, u.miq_groups.pluck(:id) ]
=> ["bbb", nil, [10000000000005]]

1. tried to login as that user in the API

error: {kind: "unauthorized", message: "Invalid User bbb specified, User's Group is missing",…}

1. checked in the console again - identical output

=> ["bbb", nil, [10000000000005]]

1. commented out the code that does the API.login on the login screen, leaving only the ops login

diff --git a/app/assets/javascripts/miq_application.js b/app/assets/javascripts/miq_application.js
index 0a503d19b..ba436e7df 100644
--- a/app/assets/javascripts/miq_application.js
+++ b/app/assets/javascripts/miq_application.js
@@ -707,11 +707,11 @@ function miqAjaxAuth(url) {
     serialized: miqSerializeForm('login_div'),
   };
 
-  return vanillaJsAPI.login(credentials.login, credentials.password)
-  .then(function() {
-    return vanillaJsAPI.ws_init();
-  })
-  .then(function() {
+//  return vanillaJsAPI.login(credentials.login, credentials.password)
+//  .then(function() {
+//    return vanillaJsAPI.ws_init();
+//  })
+//  .then(function() {
     // API login ok, now do the normal one
     miqJqueryRequest(url || '/dashboard/authenticate', {
       beforeSend: true,
@@ -719,15 +719,15 @@ function miqAjaxAuth(url) {
     });
 
     // TODO vanillaJsAPI.autorenew is called on (non-login) page load - when?
-  })
-  .then(null, function() {
-    clearFlash();
-    add_flash(__('Incorrect username or password'), 'error', { id: 'auth_failed' });
+//  })
+//  .then(null, function() {
+//    clearFlash();
+//    add_flash(__('Incorrect username or password'), 'error', { id: 'auth_failed' });
 
     miqClearLoginFields();
     miqEnableLoginFields(true);
     miqSparkleOff();
-  });
+// });
 }
 
 // Send SSO login authentication via ajax

1. tried to log in again

Login not allowed, User's Group is missing. Please contact the administrator

this time from /dashboard/authenticate.

So... OK, maybe this is not an API bug after all - sorry, I could have sworn this worked last week when I tried :(.

But it does seem to match what UserValidationService code says, so... I guess I diagnosed this part wrong, sorryy ..

[himdel]

@mzazrivec , @imtayadeway Given that Rails login doesn't work without current_group either after all, and that creating users via the API (almost) works too, should we consider it an UI bug instead, and close this issue?

Meaning ManageIQ/manageiq-ui-classic#3894 (or a similar PR) would become the fix for this.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 6 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions!

[himdel]

@miq-bot close_issue

This was resolved by fixing how new users get created - ManageIQ/manageiq-ui-classic#3917