Update role_mappings to avoid data loss

Does this update help auto-mapping new openID users to existing user's roles?

Just tested out the overnight dev version. confirm that it works

FWIW: As a work-around for those seeing permissions disappearing, adding a role mapping in the openid_connect/settings page, saving, and then removing it and saving again updates the settings to remove all empty values (as the patch does) and avoids the problem.

We didn't experience this problem until after updating to 3.0.0-alpha4. The change introduced by #3492759: Mapped user roles are not always revoked caused the issue to manifest. Since we don't use the role mapping feature they were all empty. After the restriction of checking to see if the user has groups was removed it suddenly started removing all roles from everyone upon login. The conclusion I'm drawing from this is that you may be about to have a whole lot more people reporting data loss problems to you if this isn't patched.

That said, I reviewed the patch. The changes to the configuration are pretty simple. So I tested it out on one of our broken sites. It worked to fix the issue. The changes it makes to the configuration are the same as if you follow the workaround steps in #6. So this is RTBC to me unless you want an update path test. I'd work on writing one for you, but creating the test is going to be pretty involved since you don't already have a test fixture set up and I need to go patch 15 websites now before anyone else tries to log in.

By the way, it's unnecessary to add a mapping, save, remove the mapping, and save again for the workaround. You can just resave the form once without entering anything.

The test is failing due to an issue with an externalauth view schema. I don't know why the test still fails even though I set the property to stop checking schemas. There's probably some nuance of the config checker I don't know about. But since that didn't work and it really ought to be fixed upstream I submitted #3496292: Remove default_argument_skip_url key from views.view.authmap to the externalauth module in an effort to try and move past this. Hopefully they'll verify the bug and commit the change soon.

I'm setting the status to Needs Review even though the test doesn't pass on GitLab CI so people will start looking at the test changes.

Assigning myself as I review the issue.

Let me know if you would like for me to work on any changes. I'm available to help this afternoon.

I updated the tests to:

1. Fix the externalauth schema error
2. Added a test case that ensures an existing role mapping remains.
3. Made the fixture a bit easier to read.

I believe this all looks good. Would love someone to review my changes before I commit.

Would love someone to review my changes before I commit.

Please see the above comments.

Oops, hit the submit button too soon.
 
If you don't want to take any action on what I mentioned, then this is RTBC from me. I'll go ahead and set the status.

I made the change you called out on the MR @dcam.

Thanks everyone for your work! This has been merged.

Cheers, thanks for working on this with us and getting it committed so quickly!

Though I just noticed that I didn't get credited on the commit. Oh well. Could you add issue credit for me, please?

Attributing credit to all who helped. Thanks again!