D evforce #20
D evforce #20
Conversation
|
|
||
| namespace Know_Your_Nation_Speedy.Controllers | ||
| { | ||
| public class RattingsController : Controller |
There was a problem hiding this comment.
Is this a spelling mistake?
| public class UsersController : ControllerBase | ||
| { | ||
| private readonly MyDbContext _db; | ||
|
|
|
|
||
| // GET api/values | ||
| [HttpGet] | ||
| public async Task<ActionResult<IEnumerable<User>>> Get() |
There was a problem hiding this comment.
How did this even compile?
| if (entry != null) | ||
| { | ||
| emailService.SendMail(mail, "testing", code); | ||
| _db.SaveChanges(); |
There was a problem hiding this comment.
You dont have any changes to save here?
|
|
||
| [HttpPut()] | ||
| [Route("ForgotPassword/{mail}")] | ||
| public async Task getCodes(string mail) |
There was a problem hiding this comment.
This function name is not consistent with the standards of the rest of the project and C# as a whole
There was a problem hiding this comment.
Also, why call it getCodes when in fact it sends an email?
|
|
||
| // PUT api/values/5 | ||
| [HttpPut()] | ||
| [Route("ResetPassword/{password} + {mail}")] |
There was a problem hiding this comment.
Why the + ?? it has to be a /
| [Route("ResetPassword/{password} + {mail}")] | ||
| public async Task ResetPassword(string mail,string password) | ||
| { | ||
| var entry = await _db.UserEntries.SingleOrDefaultAsync(m => m.Email == mail); |
There was a problem hiding this comment.
You need a null check and error handling here
| namespace Know_Your_Nation_Speedy.Models{ | ||
| public class EmailService | ||
| { | ||
| string smtpAddress = "smtp.gmail.com"; |
There was a problem hiding this comment.
All of these fields should be in the config
| smtp.Send(mail); | ||
| } | ||
|
|
||
| public string generateCode() |
There was a problem hiding this comment.
I dont think its necessary to write this whole function to generate a "unique" code, which, in fact, isn't unique. You can generate a GUID instead.
GUID (or UUID) is an acronym for 'Globally Unique Identifier' (or 'Universally Unique Identifier'). It is a 128-bit integer number used to identify resources. The term GUID is generally used by developers working with Microsoft technologies, while UUID is used everywhere else.
128-bits is big enough and the generation algorithm is unique enough that if 1,000,000,000 GUIDs per second were generated for 1 year the probability of a duplicate would be only 50%. Or if every human on Earth generated 600,000,000 GUIDs there would only be a 50% probability of a duplicate.
| { | ||
| using (MailMessage mail = new MailMessage()) | ||
| { | ||
| MailAssignment(mail, emailFromAddress, To, Subject, "<a href = 'http://ereader.retrotest.co.za/resetPassword'>Reset Password</h1>"); |
There was a problem hiding this comment.
You don't use the generated code here?
No description provided.