Send new Gitea account credentials via Mailtrap#187
Conversation
Add GiteaAccountCredentialsNotification using the Mailtrap channel and HTML email template. Notify users only after successful Gitea user creation (not when linking an existing account). Feature tests cover notification sent and skipped cases. Made-with: Cursor
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request implements a notification system to send Gitea account credentials to users when their accounts are provisioned. It includes a new notification class, a Blade email template, and updates to the provisioning service and tests. Feedback focuses on improving the reliability of the provisioning flow by ensuring notifications are queued before state changes are finalized, addressing security risks associated with storing plain-text passwords in the queue, and maintaining consistency between the text and HTML versions of the email.
|
|
||
| $user->forceFill(['gitea_username' => $login])->save(); | ||
|
|
||
| $user->notify(new GiteaAccountCredentialsNotification($login, $password)); |
There was a problem hiding this comment.
The notification is dispatched after the gitea_username is saved to the database. If the notification fails to be queued (e.g., due to a connection issue with the queue driver), the user will be marked as having a Gitea account in the database, but they will never receive their credentials. Subsequent retries of the provisioning process will return early because gitea_username is already set (see line 89), leaving the user with an inaccessible account. Consider swapping the order to ensure the notification is successfully queued before finalizing the state in the database.
| public string $giteaUsername, | ||
| public string $temporaryPassword, |
There was a problem hiding this comment.
The temporary password is stored as a public property on a queued notification. When the notification is queued, these properties are serialized and stored in plain text in the queue storage (e.g., Redis or the jobs table). This exposes sensitive credentials to anyone with access to the queue or logs. While it is a temporary password, consider encrypting this data or using a flow that doesn't involve sending plain-text passwords through the queue.
| 'A Gitea account has been created for you on '.config('app.name').'.', | ||
| '', | ||
| 'Username: '.$this->giteaUsername, | ||
| 'Temporary password: '.$this->temporaryPassword, | ||
| '', | ||
| 'You must change this password when you first sign in to Gitea.', |
There was a problem hiding this comment.
The introductory sentence in the text version of the email differs from the HTML version, and it is missing the greeting. It's better to keep the messaging consistent across both formats to ensure a uniform user experience.
'Hello ' . $notifiable->name . ',',
'',
'A Gitea account has been created for you so you can collaborate on remote work repositories.',
'',
'Username: ' . $this->giteaUsername,
'Temporary password: ' . $this->temporaryPassword,
'',
'You must change this password when you first sign in to Gitea.',1 participant
Summary
When
CompanyJobGiteaProvisionercreates a new Gitea user via the admin API, the user now receives their username and temporary password through the existing Mailtrap notification channel.Details
GiteaAccountCredentialsNotification(queued) with HTML templateemails/gitea-account-credentials.createUserresponse; existing users resolved by email are not emailed (no password available).Tests
php artisan test --compact tests/Feature/CompanyJobGiteaProvisionerTest.phpMade with Cursor