Address #21 with username validation

spookygames · Feb 2, 2022 · d7f7e4f · d7f7e4f
1 parent 20255f7
commit d7f7e4f
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 11 deletions.
diff --git a/LICENSE b/LICENSE
@@ -1,6 +1,6 @@
 MIT License
 
-Copyright (c) 2019 Spooky Games
+Copyright (c) 2019-2022 Spooky Games
 
 Permission is hereby granted, free of charge, to any person obtaining a copy
 of this software and associated documentation files (the "Software"), to deal

diff --git a/README.md b/README.md
@@ -45,3 +45,11 @@ In the end, extension settings will be:
 * Encryption key (or cert): you may copy here the content of what was displayed after you pressed the "Public key" button on Keycloak.
 * Role-to-group mapping: An associative array with roles as keys and group names as values, in JSON format. Example: `{"ROLE_MEMBER":"Member","ROLE_MODERATOR":"Mods","ROLE_ADMIN":"Admin"}`.
 * Delegate avatars: if enabled, the "picture" attribute from Keycloak will be used to handle user avatar instead of Flarum's default behaviour.
+
+## Troubleshooting
+
+### User created with an odd name that does not match actual user name like 'tgtplwexeowwluxnqid4cjgw' ([original issue](https://github.com/spookygames/flarum-ext-auth-keycloak/issues/21))
+
+Flarum only allows user names that match the regular expression `/[^a-z0-9-_]/i`.
+Every Keycloak user with a "preferred_username" not matching this expression will instead be assigned a random name, as well as a proper Flarum "nickname".
+In order to see the nickname instead of the random user name, activate the Nicknames extension and use the User Display Name driver named _nickname_.
diff --git a/src/KeycloakAuthController.php b/src/KeycloakAuthController.php
@@ -14,6 +14,7 @@
 use Flarum\User\Command\RegisterUser;
 use Illuminate\Contracts\Bus\Dispatcher;
 use Illuminate\Support\Arr;
+use Illuminate\Support\Str;
 use League\OAuth2\Client\Token\AccessToken;
 use Stevenmaguire\OAuth2\Client\Provider\Keycloak;
 use Stevenmaguire\OAuth2\Client\Provider\KeycloakResourceOwner;
@@ -194,7 +195,7 @@ function (Registration $registration) use ($remoteUser, $groups, $actor) {
 
                     try {
                         // Create user
-                        $created = $this->bus->dispatch(new RegisterUser($actor, $data));
+                        $created = $this->bus->dispatch(new RegisterUser($adminActor, $data));
 
                         // Edit user afterwards in order to propagate groups too
                         $this->bus->dispatch(new EditUser($created->id, $adminActor, $data));
@@ -203,9 +204,7 @@ function (Registration $registration) use ($remoteUser, $groups, $actor) {
                         // Remove its new login provider (will be re-created right afterwards)
                         $created->loginProviders()->delete();
                     } catch (Exception $e) {
-                        if ($created->id != 1) {
-                            exit('Failed to update Flarum user: '.$e->getMessage());
-                        }
+                        exit('Failed to create Flarum user: '.$e->getMessage());
                     }
 
                 }
@@ -215,14 +214,23 @@ function (Registration $registration) use ($remoteUser, $groups, $actor) {
 
    public function decorateRegistration(Registration $registration, KeycloakResourceOwner $remoteUser): Registration
    {
-        $remoteUserArray = $remoteUser->toArray();
+      $remoteUserArray = $remoteUser->toArray();
+
+      $registration->provideTrustedEmail($remoteUser->getEmail());
+
+      // Same regex used in Registration.suggestUsername
+      $rawUsername = Arr::get($remoteUserArray, 'preferred_username');
+      $username = preg_replace('/[^a-z0-9-_]/i', '', $rawUsername);
+      if ($username == $rawUsername) {
+        $registration->suggestUsername($rawUsername);
+      } else {
+        $registration->suggestUsername(Str::lower(Str::random(24)));
+        $registration->suggest('nickname', $rawUsername);
+      }
 
-       $registration
-           ->provideTrustedEmail($remoteUser->getEmail())
-           ->suggestUsername(Arr::get($remoteUserArray, 'preferred_username'))
-           ->setPayload($remoteUserArray);
+      $registration->setPayload($remoteUserArray);
 
-       return $registration;
+      return $registration;
    }
 
   public function updateInternalIfNeeded(User $user, KeycloakResourceOwner $remoteUser): User