-
Notifications
You must be signed in to change notification settings - Fork 57
Email Enforcement #1088
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Email Enforcement #1088
Changes from all commits
980a2af
fbe3cee
9e9c96e
7495c41
cb6d772
5bdb1c0
f6cbafd
67a8c6d
a322a32
561e1d4
91c5976
9732936
64e21ab
0c8ca73
479b1da
3041d2c
3fb598e
048f60c
f77e5ee
3fb1441
40a4d5e
0d3e139
ed9c146
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -26,5 +26,10 @@ public static class BaseLayoutStrings | |
| public static readonly TranslatableString ReadOnlyWarnTitle = create("read_only_warn_title"); | ||
| public static readonly TranslatableString ReadOnlyWarn = create("read_only_warn"); | ||
|
|
||
| public static readonly TranslatableString EmailEnforcementWarnMain = create("email_enforcement_message_main"); | ||
| public static readonly TranslatableString EmailEnforcementWarnNoEmail = create("email_enforcement_message_no_email"); | ||
| public static readonly TranslatableString EmailEnforcementWarnVerifyEmail = create("email_enforcement_message_verify_email"); | ||
|
|
||
|
Comment on lines
+29
to
+32
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You only use these strings within GameServer code which doesn't have localization support and will therefore always be in the default language. This should probably just be hardcoded in the Announce method. |
||
|
|
||
| private static TranslatableString create(string key) => new(TranslationAreas.BaseLayout, key); | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,7 +42,7 @@ | |
| You should have received a copy of the GNU Affero General Public License | ||
| along with this program. If not, see <https://www.gnu.org/licenses/>."; | ||
|
|
||
| public MessageController(DatabaseContext database) | ||
| { | ||
| this.database = database; | ||
| } | ||
|
|
@@ -55,16 +55,31 @@ | |
| { | ||
| GameTokenEntity token = this.GetToken(); | ||
|
|
||
| string username = await this.database.UsernameFromGameToken(token); | ||
| UserEntity? user = await this.database.UserFromGameToken(token); | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (user == null) return this.BadRequest(); | ||
|
|
||
| StringBuilder announceText = new(ServerConfiguration.Instance.AnnounceText); | ||
|
|
||
| announceText.Replace("%user", username); | ||
| announceText.Replace("%user", user.Username); | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| announceText.Replace("%id", token.UserId.ToString()); | ||
|
|
||
| if (ServerConfiguration.Instance.UserGeneratedContentLimits.ReadOnlyMode) | ||
| { | ||
| announceText.Insert(0, BaseLayoutStrings.ReadOnlyWarn.Translate(LocalizationManager.DefaultLang) + "\n\n"); | ||
| announceText.Append(BaseLayoutStrings.ReadOnlyWarn.Translate(LocalizationManager.DefaultLang) + "\n\n"); | ||
| } | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (ServerConfiguration.Instance.EmailEnforcement.EnableEmailEnforcement) | ||
| { | ||
| announceText.Append("\n\n" + BaseLayoutStrings.EmailEnforcementWarnMain.Translate(LocalizationManager.DefaultLang) + "\n\n"); | ||
|
|
||
| if (user.EmailAddress == null) | ||
| { | ||
| announceText.Append(BaseLayoutStrings.EmailEnforcementWarnNoEmail.Translate(LocalizationManager.DefaultLang) + "\n\n"); | ||
| } | ||
| else if (!user.EmailAddressVerified) | ||
| { | ||
| announceText.Append(BaseLayoutStrings.EmailEnforcementWarnVerifyEmail.Translate(LocalizationManager.DefaultLang) + "\n\n"); | ||
| } | ||
| } | ||
|
|
||
| #if DEBUG | ||
|
|
@@ -72,7 +87,7 @@ | |
| $"user.UserId: {token.UserId}\n" + | ||
| $"token.GameVersion: {token.GameVersion}\n" + | ||
| $"token.TicketHash: {token.TicketHash}\n" + | ||
| $"token.ExpiresAt: {token.ExpiresAt.ToString()}\n" + | ||
| "---DEBUG INFO---"); | ||
| #endif | ||
|
|
||
|
|
@@ -128,13 +143,13 @@ | |
|
|
||
| if (message.StartsWith("/setemail ") && ServerConfiguration.Instance.Mail.MailEnabled) | ||
| { | ||
| string email = message[(message.IndexOf(" ", StringComparison.Ordinal)+1)..]; | ||
|
Check notice on line 146 in ProjectLighthouse.Servers.GameServer/Controllers/MessageController.cs
|
||
| if (!SanitizationHelper.IsValidEmail(email)) return this.Ok(); | ||
|
|
||
| if (await this.database.Users.AnyAsync(u => u.EmailAddress == email)) return this.Ok(); | ||
| // Return a bad request on invalid email address | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| if (!SMTPHelper.IsValidEmail(this.database, email)) return this.BadRequest(); | ||
|
|
||
| UserEntity? user = await this.database.UserFromGameToken(token); | ||
| if (user == null || user.EmailAddressVerified) return this.Ok(); | ||
| if (user == null || user.EmailAddressVerified) return this.BadRequest(); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Have you tested that the game doesn't behave erratically if the request fails? The purpose of returning an empty 200 response is so that in a multiplayer lobby, players won't be able to see the user's email that they just typed.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have not tested this with other clients, but I would assume that if the response is empty then the clients shouldn't get the message either way. I can try testing this with a proxy or something since I don't necessarily have multiple devices I can run clients on to host a lobby.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The client sending the message seems to function perfectly fine when it gets the 400 back though. |
||
|
|
||
| user.EmailAddress = email; | ||
| await SMTPHelper.SendVerificationEmail(this.database, mailService, user); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,62 @@ | ||
| using LBPUnion.ProjectLighthouse.Configuration; | ||
| using LBPUnion.ProjectLighthouse.Database; | ||
| using LBPUnion.ProjectLighthouse.Middlewares; | ||
| using LBPUnion.ProjectLighthouse.Types.Entities.Profile; | ||
| using LBPUnion.ProjectLighthouse.Types.Entities.Token; | ||
|
|
||
| namespace LBPUnion.ProjectLighthouse.Servers.GameServer.Middlewares; | ||
|
|
||
| public class EmailEnforcementMiddleware : MiddlewareDBContext | ||
| { | ||
| private static readonly HashSet<string> enforcedPaths = ServerConfiguration.Instance.EmailEnforcement.BlockedEndpoints; | ||
|
|
||
| public EmailEnforcementMiddleware(RequestDelegate next) : base(next) | ||
| { } | ||
|
|
||
| public override async Task InvokeAsync(HttpContext context, DatabaseContext database) | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| { | ||
| if (ServerConfiguration.Instance.EmailEnforcement.EnableEmailEnforcement) | ||
| { | ||
| // Split path into segments | ||
| string[] pathSegments = context.Request.Path.ToString().Split("/", StringSplitOptions.RemoveEmptyEntries); | ||
|
|
||
| if (pathSegments[0] == "LITTLEBIGPLANETPS3_XML") | ||
| { | ||
| // Get user via GameToken | ||
| GameTokenEntity? token = await database.GameTokenFromRequest(context.Request); | ||
| UserEntity? user = await database.UserFromGameToken(token); | ||
|
|
||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Check second part of path to see if client is within an enforced path | ||
| // This could probably be reworked, seeing as you may want to check for a deeper sub-path | ||
| // But it should be perfectly fine for now | ||
| if (enforcedPaths.Contains(pathSegments[1])) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't look like it will handle arguments in the path. Take |
||
| { | ||
| // Check if user is valid, don't want any exceptions | ||
| if (user == null) | ||
| { | ||
| // Send bad request status | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| context.Response.StatusCode = StatusCodes.Status403Forbidden; | ||
| await context.Response.WriteAsync("Not a valid user"); | ||
|
|
||
| // Don't go to next in pipeline | ||
| return; | ||
| } | ||
|
|
||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // Check if email is there and verified | ||
| if (!user.EmailAddressVerified || user.EmailAddress == null) | ||
| { | ||
| // Send bad request status | ||
| context.Response.StatusCode = StatusCodes.Status400BadRequest; | ||
| await context.Response.WriteAsync("Invalid user email address"); | ||
FeTetra marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| // Don't go to next in pipeline | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Go to next in pipeline | ||
| await this.next(context); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,7 +18,7 @@ | |
|
|
||
| public string? Status { get; private set; } | ||
|
|
||
| public PasswordResetRequestForm(DatabaseContext database, IMailService mail) : base(database) | ||
| { | ||
| this.Mail = mail; | ||
| } | ||
|
|
@@ -38,9 +38,9 @@ | |
| return this.Page(); | ||
| } | ||
|
|
||
| if (!SanitizationHelper.IsValidEmail(email)) | ||
| if (!SMTPHelper.IsValidEmail(this.Database, email)) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It might be worth refactoring the email syntax checking back out so that you can give the user proper feedback for why the email is invalid. Otherwise, people may be confused when their email is "invalid" when in reality their domain has been blacklisted. |
||
| { | ||
| this.Error = "This email is in an invalid format"; | ||
| this.Error = "This email is invalid"; | ||
| return this.Page(); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,7 +136,7 @@ | |
| messageController.SetupTestController(request); | ||
|
|
||
| CensorConfiguration.Instance.UserInputFilterMode = FilterMode.Asterisks; | ||
| CensorConfiguration.Instance.FilteredWordList = new List<string> | ||
| { | ||
| "bruh", | ||
| }; | ||
|
|
@@ -167,7 +167,7 @@ | |
| messageController.SetupTestController(request); | ||
|
|
||
| ServerConfiguration.Instance.Mail.MailEnabled = false; | ||
| CensorConfiguration.Instance.FilteredWordList = new List<string>(); | ||
|
|
||
| const string expected = "/setemail unittest@unittest.com"; | ||
|
|
||
|
|
@@ -203,7 +203,7 @@ | |
| [Fact] | ||
| public async Task Filter_ShouldNotSendEmail_WhenMailEnabled_AndEmailTaken() | ||
| { | ||
| List<UserEntity> users = new() | ||
| { | ||
| new UserEntity | ||
| { | ||
|
|
@@ -224,7 +224,7 @@ | |
|
|
||
| IActionResult result = await messageController.Filter(mailMock.Object); | ||
|
|
||
| Assert.IsType<OkResult>(result); | ||
| Assert.IsType<BadRequestResult>(result); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comment on
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It doesn't seem to negatively affect the client in any unintended way. Any time it tries to access an endpoint that is blocked, the game complains that it is having trouble connecting to the servers. |
||
| mailMock.Verify(x => x.SendEmailAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never); | ||
| } | ||
|
|
||
|
|
@@ -249,7 +249,7 @@ | |
|
|
||
| IActionResult result = await messageController.Filter(mailMock.Object); | ||
|
|
||
| Assert.IsType<OkResult>(result); | ||
| Assert.IsType<BadRequestResult>(result); | ||
| mailMock.Verify(x => x.SendEmailAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never); | ||
| } | ||
|
|
||
|
|
@@ -271,7 +271,7 @@ | |
|
|
||
| IActionResult result = await messageController.Filter(mailMock.Object); | ||
|
|
||
| Assert.IsType<OkResult>(result); | ||
| Assert.IsType<BadRequestResult>(result); | ||
| mailMock.Verify(x => x.SendEmailAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never); | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should probably be hardcoded since they're not used in the website.