brockallen / BrockAllen.MembershipReboot

MembershipReboot is a user identity management and authentication library.
Other
742 stars 238 forks source link

Account confirmation not working #600

Closed AnMVCCoder closed 8 years ago

AnMVCCoder commented 8 years ago

I've been in the process of implementing this library, at first with not so much success, but have been gaining traction, and have had some success. I went from (all within my own custom-hand made project with the use of membershipreboot's nuget packages, and taking a little bit of the SingleTenant's example code for a little bit of models, views, and the ChangeEmailController) it not working at all, to having di issues. I fixed all that (with a little help). Now, the login and register pages work, and registration works as well as sending out verification emails, they all work. This is where I'm now stuck at. After receiving the verification email, and clicking the link in the email - I go to the localhost:####/ChangeEmail/(verification key) page - and this is where my new problem is. I'm using some of the code taken from the (SingleTenant) sample code.

When it goes to that page, in the default setup, it told me it had "System.ArgumentException: account" and it pointed to the code "if (account.HasPassword())" in the Confirm ActionResult of the ChangeEmailController. I added a line of code above that if statement in that ActionResult - "Redirect("http://" + account.Email);" - just to see what it would do with that. With that addition, it gave me another (related) error - "Object reference not set to an instance of an object." and it pointed to that exact code I added.

So, it's obvious that the system works in loading the library, it works in being able to actually register a user, it works in being able to send an email with the proper verification key (I compared it to the database entry, it was the same code). But, then when the link to the verification method is clicked on - it doesn't seem to want to retrieve that same account data from the database.

I also made a post on stacoverflow about the issue - http://stackoverflow.com/questions/34459825/brockallen-membershipreboot-account-confirmation-not-working

brockallen commented 8 years ago

So I'd suggest setting up the trace config to see the log file. This might help you diagnose how far in the code it gets and then where/why it starts to go south.

AnMVCCoder commented 8 years ago

Thanks for the quick response. But, how do I set up the trace config?

AnMVCCoder commented 8 years ago

Well, I tried one method of tracing, from what I found in https://msdn.microsoft.com/en-us/library/wwh16c6c.aspx - and putting this node in the system.web section of web.config - - and got information through the trace.axd file - this information to me, kind of seems useless as to helping me "diagnose how far in the code it gets and then where/why it starts to go south". But, I don't know, clearly you have more knowledge in programming than I do, maybe you might find some of it of use, but I kind of doubt it from what I see, but who knows. This is the most interesting information I got from trace.axd:

ALL_HTTP:
HTTP_CACHE_CONTROL:max-age=0 HTTP_CONNECTION:keep-alive HTTPACCEPT:text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,/_;q=0.8 HTTP_ACCEPT_ENCODING:gzip, deflate, sdch HTTP_ACCEPT_LANGUAGE:en-US,en;q=0.8 HTTP_COOKIE:__RequestVerificationToken=IlPAmmS0DEVgvKB-RIOVk2XXJddG3ty3CY1pMvfTVuMCu7TLyumsyk2_vE-TzXDw7MnoFpfKNFYB3Mvsn7rm_AFQEumTYOBW6c_pbJwKxGA1 HTTP_HOST:localhost:5613 HTTP_USER_AGENT:Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36 HTTP_UPGRADE_INSECURE_REQUESTS:1

ALLRAW: Cache-Control: max-age=0 Connection: keep-alive Accept: text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,/_;q=0.8 Accept-Encoding: gzip, deflate, sdch Accept-Language: en-US,en;q=0.8 Cookie: __RequestVerificationToken=IlPAmmS0DEVgvKB-RIOVk2XXJddG3ty3CY1pMvfTVuMCu7TLyumsyk2_vE-TzXDw7MnoFpfKNFYB3Mvsn7rm_AFQEumTYOBW6c_pbJwKxGA1 Host: localhost:5613 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/47.0.2526.106 Safari/537.36 Upgrade-Insecure-Requests: 1

APPL_MD_PATH: /LM/W3SVC/2/ROOT

But, I must add, I looked online for tracing methods for MVC, and there seems like there're a few options, but I don't know which one in particular would provide the best data. I also know about Profiling (like RedGate tools). But, I don't know which would provide the best details in these regards, so again, I ask, what do you suggest when it comes to tracing? If you want, I can upload my project to a download source, and you can download it and see it for yourself, I'll do whatever it is you ask in order to help me fix this issue. I really do like MembershipReboot, and I know, with your help, this matter will be resolved.

AnMVCCoder commented 8 years ago

Or, in the mean time, in waiting for your response, maybe the stack trace information can come handy, I don't know. Well here it is:

[NullReferenceException: Object reference not set to an instance of an object.] BrockAllen.MembershipReboot.Mvc.Areas.UserAccount.Controllers.ChangeEmailController.Confirm(String id) in C:\Users\Big Pill_-\Documents\Visual Studio 2015\Projects\brockallen_MembershipReboot_WithoutExtraSamplesCode\brockallen_MembershipReboot\Controllers\ChangeEmailController.cs:63 lambda_method(Closure , ControllerBase , Object[] ) +104 System.Web.Mvc.ActionMethodDispatcher.Execute(ControllerBase controller, Object[] parameters) +14 System.Web.Mvc.ReflectedActionDescriptor.Execute(ControllerContext controllerContext, IDictionary2 parameters) +157 System.Web.Mvc.ControllerActionInvoker.InvokeActionMethod(ControllerContext controllerContext, ActionDescriptor actionDescriptor, IDictionary2 parameters) +27 System.Web.Mvc.Async.AsyncControllerActionInvoker.b39(IAsyncResult asyncResult, ActionInvocation innerInvokeState) +22 System.Web.Mvc.Async.WrappedAsyncResult2.CallEndDelegate(IAsyncResult asyncResult) +29 System.Web.Mvc.Async.WrappedAsyncResultBase1.End() +49 System.Web.Mvc.Async.AsyncControllerActionInvoker.EndInvokeActionMethod(IAsyncResult asyncResult) +32 System.Web.Mvc.Async.AsyncInvocationWithFilters.b3d() +50 System.Web.Mvc.Async.<>cDisplayClass46.b3f() +225 System.Web.Mvc.Async.<>cDisplayClass33.b32(IAsyncResult asyncResult) +10 System.Web.Mvc.Async.WrappedAsyncResult1.CallEndDelegate(IAsyncResult asyncResult) +10 System.Web.Mvc.Async.WrappedAsyncResultBase1.End() +49 System.Web.Mvc.Async.AsyncControllerActionInvoker.EndInvokeActionMethodWithFilters(IAsyncResult asyncResult) +34 System.Web.Mvc.Async.<>cDisplayClass2b.b1c() +26 System.Web.Mvc.Async.<>cDisplayClass21.b1e(IAsyncResult asyncResult) +100 System.Web.Mvc.Async.WrappedAsyncResult1.CallEndDelegate(IAsyncResult asyncResult) +10 System.Web.Mvc.Async.WrappedAsyncResultBase1.End() +49 System.Web.Mvc.Async.AsyncControllerActionInvoker.EndInvokeAction(IAsyncResult asyncResult) +27 System.Web.Mvc.Controller.b1d(IAsyncResult asyncResult, ExecuteCoreState innerState) +13 System.Web.Mvc.Async.WrappedAsyncVoid1.CallEndDelegate(IAsyncResult asyncResult) +29 System.Web.Mvc.Async.WrappedAsyncResultBase1.End() +49 System.Web.Mvc.Controller.EndExecuteCore(IAsyncResult asyncResult) +36 System.Web.Mvc.Controller.b15(IAsyncResult asyncResult, Controller controller) +12 System.Web.Mvc.Async.WrappedAsyncVoid1.CallEndDelegate(IAsyncResult asyncResult) +22 System.Web.Mvc.Async.WrappedAsyncResultBase1.End() +49 System.Web.Mvc.Controller.EndExecute(IAsyncResult asyncResult) +26 System.Web.Mvc.Controller.System.Web.Mvc.Async.IAsyncController.EndExecute(IAsyncResult asyncResult) +10 System.Web.Mvc.MvcHandler.b__5(IAsyncResult asyncResult, ProcessRequestState innerState) +21 System.Web.Mvc.Async.WrappedAsyncVoid1.CallEndDelegate(IAsyncResult asyncResult) +29 System.Web.Mvc.Async.WrappedAsyncResultBase1.End() +49 System.Web.Mvc.MvcHandler.EndProcessRequest(IAsyncResult asyncResult) +28 System.Web.Mvc.MvcHandler.System.Web.IHttpAsyncHandler.EndProcessRequest(IAsyncResult result) +9 System.Web.CallHandlerExecutionStep.System.Web.HttpApplication.IExecutionStep.Execute() +576 System.Web.HttpApplication.ExecuteStep(IExecutionStep step, Boolean& completedSynchronously) +157

AnMVCCoder commented 8 years ago

Actually, the thing is, I have it set up and working with ninject, and I prefer lightinject, so I have it set up and working in the exact same way with both di's, and the app has the same error whether it's ninject or lightinject, but, when the system is set up with one or the other di, the stack trace is actually different (it's a noticeably longer list with ninject, from what I can see), the stack trace I posted earlier was from the version of the app with lightinject. Also, from what I've seen, the stack trace seemed to have been a longer list of information when I implemented another method for tracing shown to me through Microsoft. I was intending to include all of this in this message, but I looked at the time, and it's getting late, and I have work tomorrow so I can get some extra overtime with holiday pay at my day job, Happy Holidays by the way. Anyway, I assume you'll message back before I'll be back from work tomorrow, I will do what you suggest, and respond accordingly. Again, thank you.

brockallen commented 8 years ago

The .NET trace mechanism is used for logging. One of the samples have the trace listener configured in web.config -- you can look for that for how to set it up.

Ah, here it is:

https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/samples/SingleTenant/SingleTenantWebApp/Web.config#L167

AnMVCCoder commented 8 years ago

Ok, so, I got some trace data to show you (directly from the log file, in xml form). But first let me say, to me it kind of makes no sense, but since this is your project, I'm sure it's more than likely it'd make more sense to you. Well, the thing is, when it does a call to userAccountService.GetByVerificationKey - it does what it's supposed to, and then the trace reports "[userAccountService.GetByVerificationKey] failed to locate account: (a long id number)" - and digging deeper, going into your sample code for the single tenant project, I see that GetByVerificationKey in UserAcccountService has this line of code - "key = this.Configuration.Crypto.Hash(key);" - so, this might explain my confusion. You see, what confuses me is that when it says "failed to locate account: (a long id number)" - that "long id number" - is different from the id number I see in the database. Also, in the method GetByVerificationKey in UserAcccountService in the single tenant project, this error is displayed after another check on "var account = userRepository.GetByVerificationKey(key);" and if "(account == null)" - which is not the case, the account exists, and that verification key provided is the exact key in the account when looking in the database. So, obviously this has me scratching my head. Hopefully, you can provide some kind of tip that'd fix this for me based on all this information. As I said, I can give you a link to the project, after I upload it to some download place online, and you can see for yourself. I have Visual Studio 2015 Enterprise, if that helps.

Anyway, these are the two trace nodes specifically associated with that request (there aren't any other nodes associated with that request) (since it won't let me to include direct xml, from what I can tell, I formatted it, but you can tell it came from the trace, and it has the important data that you'd expect):

E2ETraceEvent xmlns="http://schemas.microsoft.com/2004/06/E2ETraceEvent" EventID: 0 Type: 3 SubType Name="Information": 0 Level: 8 TimeCreated SystemTime="2015-12-27T02:31:41.2917739Z" Source Name="MembershipReboot" Correlation ActivityID="{80000011-0001-f600-b63f-84710c7967bb}" Execution ProcessName="iisexpress" ProcessID="14088" ThreadID="7" Channel/ Computer: (mypc) ApplicationData: [UserAccountService.GetByVerificationKey] called for key: 91F3F7554616559526146F9CBEC3AA1BD18CE7AB513E9D55E459280A6536C7B0 /E2ETraceEvent E2ETraceEvent xmlns="http://schemas.microsoft.com/2004/06/E2ETraceEvent" EventID: 0 Type: 3 SubType Name="Warning": 0 Level: 4 TimeCreated SystemTime="2015-12-27T02:31:41.8252309Z" Source Name="MembershipReboot" Correlation ActivityID="{80000011-0001-f600-b63f-84710c7967bb}" Execution ProcessName="iisexpress" ProcessID="14088" ThreadID="7" Channel/ Computer: (mypc) ApplicationData: [UserAccountService.GetByVerificationKey] failed to locate account: 7A5F053864FC699211B4CF159A4BA900E4273BB191247862FAEA736D7065597D /E2ETraceEvent

AnMVCCoder commented 8 years ago

And by the way, obviously you are a very well experienced programmer, to make such an ambitious goal of making a security project that is more advanced than what Microsoft is offering, you clearly are an expert. I know there must be something here that I must've done wrong, though it is not at all close to obvious to me what that may be, and I'm sure after some more back and forth, you'll be able to help me. I, and surely all the other developers out there that are using membershipreboot are clearly thankful for your work, and your willingness to help us when we have such problems.

brockallen commented 8 years ago

The verification key is hashed so that if the DB is compromised that the attacker can't use the confirmation codes.

As for it not matching -- maybe case sensitivity in the DB query?

AnMVCCoder commented 8 years ago

Ok, well, I didn't respond till now, because it was hard for me to think of how do I put this. I don't know what to make of what you just said, I didn't alter the db query, so I don't know how that makes sense. I decided I can always make my own hack-fix - by creating my own method doing the exact same thing. But, it's just quite a bit troubling to see that this is even occurring. It's even more troubling to see that upon doing various searches through google on this issue, and not one, from what I can see, other person has even reported this happening. I don't know why I seem to be the only person on the internet that has encountered this problem. But, I assure you, I did not change your code, and am strictly relying on your library (in it's most up-to-date Nuget packages) as it's supposed to be implemented. As I said, I can upload my project to the internet, and you can download it, and see for yourself. I don't have a clue why this is happening, and it just doesn't make any sense that it's happening. Yes, I can hack it and create a custom method, but first, I hope I can get a more accurate response from you. Clearly something strange is going on here, and I just hope you can help me in these regards.

AnMVCCoder commented 8 years ago

Ok, since you didn't respond, I guess I'll have no choice but to make verification happen with the system with my own code I write (what I previously called a "hack-fix") that does what membershipreboot is supposed to be doing, but for some reason doesn't. You're explanation is odd though, I mean you are the author of this library, and you don't have any interest as to why such weird behaviour is occurring for me? All you offer is - maybe - "case sensitivity in the DB query"? I didn't write the query, you did. But I digress, I'm not even sure if membershipreboot is such a great idea anymore, seeing as how there are over 600 (although, I'm pretty sure many of them have been addressed seeing as it is at version 8 (I think it was)) (isn't it possible that verification has been somehow broken on the newest version you released to nuget, and you weren't aware of it? (which was kind of the reason why I said "and you don't have any interest as to why such weird behaviour is occurring for me?")) issues mentioned by those that have or have tried - to implement it. But, I'm a risk taker by nature, so I say, ok, I'll give it a shot, overlooking this problem I originally was talking about, as well as the concerns I just raised (among others). I'm not trying to sound rude towards you, I just thought your behaviour has been just a little odd here. But, I have a lot of respect for you, and it's unfortunate you're not as willing to help me here as I wish you would.

But since you chose not to respond to my previous most recent comment before this one, and so I obviously guess what you expect me to do is figure it out, and leave you alone on this matter.

I just have one new question on this matter before I do that, and please let me know as good of an answer as you can on the matter:

The question: So, if I do create my own custom code (EntityFramework based mainly) that does the same thing membershipreboot should be doing when it comes to verification, will it in any way effect the functionality of membershipreboot, are there any negatives to this approach? Please let me know, thank you.

PS: I just remembered, at first I wasn't even able to make membershipreboot run through the nuget package in my own project (which I figured out how to do a few days later), and I was at that time checking out the sample projects, and upon registering a user with the singletenant project, and receiving the email - and by the way, that's another thing, you're default email mechanism doesn't seem to work either, I had to add my own code for it, but that's fine... Anyway, once the email was received in that situation, I went to the verification (for the singletentant project) link in the email - and it too had a problem - with a different error message, but there was problem there too. After that I was hoping it wouldn't happen with my custom project, but as I said - it does - just with a different error message.

brockallen commented 8 years ago

I'm sorry you had problems. I do have a day job and have to attend to those meddlesome burdens. That means I can't spend the time on your schedule to address your issues. I wish there were more time in the day, as I'd love to be able to help. I really would. If you have more patience then I can get to it eventually, but if you need immediate support then pick a library where you can hire someone to help. Or build one yourself.

AnMVCCoder commented 8 years ago

I understand where you're coming from, I too have a busy life and day job. Ok, well, as I said, I guess I'll create my own mechanism for verification. But, you didn't address my question on the matter, will me implementing a custom mechanism for verification have any negative side-effect on the membershipreboot functionality? All I'm thinking about is receiving the verification key in the controller, adding my own custom model and entityframework code that looks at the database and tells the database this user has been verified upon receiving the verification code. Just please answer that question. And if you can help me to do it through membershipreboot instead of my custom code on a future date, on your own time, I definitely have the patience for that. I have an ambitious idea to create a website kind of quickly in the next couple months, and I wanted a comments section - where users can register, login, and comment, and have the abilities they'd expect from a standard system of this kind. I also wanted it to be as secure as possible, not just for the website itself, but so I can have a better understanding of website security than I ever did. I also want to take the Microsoft Certification Test, and become a Certified developer within the new year so I can get a job as a developer. And yeah, security is not the only thing when it comes MVC development that I need to understand, but it still is (obviously) a good asset to have knowledge in. Thank you for creating the library, and being willing to help people like me when you do have time in your busy life. Though it's not 2016 yet, I say Happy New Year to you - and probably will say it again when the year has changed.

brockallen commented 8 years ago

The verification key is hashed so that if the DB is compromised that the attacker can't use the confirmation codes. As for it not matching -- maybe case sensitivity in the DB query?

Ok, well, I didn't respond till now, because it was hard for me to think of how do I put this. I don't know what to make of what you just said, I didn't alter the db query, so I don't know how that makes sense.

So this verification key is what's sent to a user. It's the key that proves the owner of the email account received the email. You don't want this stored in the DB in plaintext, as an attacker with read access to the DB could then lift the value and resubmit it to compromise the system. This is more of a threat around password reset, rather than email verification, but it's the same column in the DB so for both scenarios the value is hashed.

So, if the key being sent is not matching, then either the DB query is case sensitive (I don't know what DB you're using or how it's configured and that was my first guess), or there's simply no match. There could be no match because you generated a new key and overwrote the prior value in the DB (for example if you tried to re-send the verification key, or tried to register a new email). This would be triggered by your code so you'd need to watch your DB to see why it changed from the time it was generated and sent to the time you tried to use it to verify the account.

Do the samples work for you as-is? Usually that's what people do -- get those working as expected, then figure out the deltas from the samples to your code.

From your prior issues above, it sounds like the verification key not working was not your original issue. I guess you solved the null reference exception?

As for the rest of it all, I'll basically don't know what the issue is and I can't tell from the info you've posted. I've tried to give the hints that I'd use to try to track down the problem. As for the last question -- I can't recommend altering or overriding aspects of MR unless you fully understand what MR is doing.

Ok, since you didn't respond, I guess I'll have no choice but to

This and the other comments that exhibit a sort of blackmail-like passive-aggressive tone to force my hand is the main reason for my frustration expressed in the above post. Perhaps I misread them.

I'll admit there is so much ponderous text that it makes it difficult to wade thru the information. Maybe others respond better to that verbosity level, but it's simply impedance for me trying to understand the issue and provide help. I'd recommend concise questions without the conversational aspects (such as "Happy New Year").

So have a Happy New Year.

AnMVCCoder commented 8 years ago

I said Happy New Year because I'm a human being, and I'm talking to you, another human being, and it turns out at the time of the writing, it was New Years, and since I say Happy New Years to everybody, even strangers, I said it to you too. You said I wrote a lot of words, but so did you in your response, but that's fine, you gave some good insights into your thinking on the matter.

Anyways, so, you wrote "Do the samples work for you as-is? Usually that's what people do -- get those working as expected, then figure out the deltas from the samples to your code." But, that was after I already wrote in a previous comment " I just remembered, at first I wasn't even able to make membershipreboot run through the nuget package in my own project (which I figured out how to do a few days later), and I was at that time checking out the sample projects, and upon registering a user with the singletenant project, and receiving the email - and by the way, that's another thing, you're default email mechanism doesn't seem to work either, I had to add my own code for it, but that's fine... Anyway, once the email was received in that situation, I went to the verification (for the singletentant project) link in the email - and it too had a problem - with a different error message, but there was problem there too. After that I was hoping it wouldn't happen with my custom project, but as I said - it does - just with a different error message." - so - I already addressed that question.

You also wrote "So, if the key being sent is not matching, then either the DB query is case sensitive (I don't know what DB you're using or how it's configured and that was my first guess), or there's simply no match." Good point, I never gave you that information, and yeah, I see from your samples, there's more than one option in these regards. Although, on your documentation wiki, you did specifically suggest using the SQL DB default, and actually, that is what I'm using, but I never did clarify that till now. I have SQL Server 2014 Express (with Advanced Services) installed on my computer. The DB itself was auto-configured upon first build of the Project - which I think had something to do with the code "Database.SetInitializer(new MigrateDatabaseToLatestVersion<DefaultMembershipRebootDatabase, BrockAllen.MembershipReboot.Ef.Migrations.Configuration>());" - but I'm not sure of that, all I know is it was auto-configured, and I'm sure it's the standard DB from your Project, it has these Tables in it "_MigrationHistory, GroupChilds, Groups, LinkedAccountClaims, LinkedAccounts, PasswordResetSecrets, TwoFactorAuthTokens, UserAccounts, UserCertificates, and - UserClaims".

In that same paragraph I just referenced, you continued to say "...or there's simply no match. There could be no match because you generated a new key and overwrote the prior value in the DB (for example if you tried to re-send the verification key, or tried to register a new email). This would be triggered by your code so you'd need to watch your DB to see why it changed from the time it was generated and sent to the time you tried to use it to verify the account." I'm pretty sure I'm not overwriting the original key, because, upon account creation, when it comes to the code that sends the verification email, the way it retrieves the verification key is like "var vk = account.VerificationKey;" (and the variable vk is included in the text of the body of the email message), the "account" variable I used in that statement is the same one that creates the account "var account = _userAccountService.CreateAccount(model.Username, model.Password, model.Email);". You also said "So this verification key is what's sent to a user. It's the key that proves the owner of the email account received the email. You don't want this stored in the DB in plaintext, as an attacker with read access to the DB could then lift the value and resubmit it to compromise the system. This is more of a threat around password reset, rather than email verification, but it's the same column in the DB so for both scenarios the value is hashed." And I already said in a previous comment "proper verification key (I compared it to the database entry, it was the same code)." - so I don't know how this hashing works, but it is the exact same key in the VerificationKey column of the UserAccounts Table - the exact same key that is sent in the email upon registration, same case, same characters, everything.

Now, I'm unsure if what I'm going to say here is a valid point, but maybe it is, or maybe it isn't, it's just an observation based on the behaviour, and I may be wrong in my observation. Maybe it is the hashing causing the inability of membershipreboot's mechanism of verifying the account to work, it not being able to match the verification key provided through the email link with the one from the database. As can be seen in the information I provided from tracing - which wrote " [UserAccountService.GetByVerificationKey] called for key: 91F3F7554616559526146F9CBEC3AA1BD18CE7AB513E9D55E459280A6536C7B0", but then also wrote (in the trace right after it) - "[UserAccountService.GetByVerificationKey] failed to locate account: 7A5F053864FC699211B4CF159A4BA900E4273BB191247862FAEA736D7065597D" - and that id code ("7A5F053864FC699211B4CF159A4BA900E4273BB191247862FAEA736D7065597D") in the failed trace message - does not match up with the verification key, or the ID column for the UserAccount Table. And when I wrote (at the beginning of this paragraph) "Maybe it is the hashing causing the inability of membershipreboot's mechanism of verifying the account to work" - what I'm implying (to be specific) is - I send the actual raw verification key entered into the DB - retrieving it from the account variable associated with creating the account - through the email - and it's included in the link. When, in my inbox, I see the email, and click the link, and it sends that data with the raw key provided in the link, and then tries to find it in the DB, you see, again, I'm unsure of how exactly your hashing mechanism works, but, maybe (a guess from my part) - what the method is expecting here is a hashed version of the verification key, not the raw version (which is what I sent in the email with the link) - and it receives the raw version, and since it's expecting the hashed version, it actually doesn't see a match because it's the raw version it received rather than the hashed version it was expecting. Again, maybe I'm wrong, but I'm just going out on a limb, and making a guess.

I write a lot of words so that you can have as much information about this as possible. I tried to address as much of what you brought up in your previous comment as possible. I still hope you can help me. It never was my intent to have a "blackmail-like passive-aggressive tone", I just was hoping you can help me, I do not by any means want to sound rude, I'm sorry if I came off that way.

brockallen commented 8 years ago

I write a lot of words so that you can have as much information about this as possible.

This is why I'm slow to respond. It takes a lot of time to parse and attempt to make sense of it all given the verbosity level.

Back to your question/issue -- From all the other stuff you just said, it sounds like perhaps what's being sent in the email is the wrong value. Are you sending the hashed value? If so, that's the wrong value. You need to email the user the verification key from the account created event. This is the property to send: https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/src/BrockAllen.MembershipReboot/AccountService/UserAccountEvents.cs#L24

That's what the default email implementation does: https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/src/BrockAllen.MembershipReboot/Notification/Email/EmailNotificationEventHandlers.cs#L102

The sample doesn't send emails at all -- if you notice, emails are just written to disk. See this line of config: https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/samples/SingleTenant/SingleTenantWebApp/Web.config#L155

I suspect all of this originated because you couldn't get the sample to work. So your changes to send the email is where the original error arose (by sending the wrong value). Just the latest guess.

AnMVCCoder commented 8 years ago

Ok, that is some interesting information. Thank you. But, I'm trying to figure out how do I actually implement it, I mean, as I said, I create the account with "var account = _userAccountService.CreateAccount(model.Username, model.Password, model.Email);", and then, I had attempted to get the proper verification key with "var vk = account.VerificationKey;" - but you're now telling me to get it through the "AccountCreatedEvent" - which contains the proper verification key I should be sending in the email. Ok, that's really promising, but, my question now is - how do I get the verification key from the AccountCreatedEvent - in a way that's similar to my previous attempt at retrieving the verification key ("var vk = account.VerificationKey;") (maybe something like "var vk = AccountCreatedEvent.VerificationKey;" - which obviously is not it (I tried something like it...) - but you know what I mean - how do I get the key and put it in a variable object like that so I can easily put it in the text of an email body)?

brockallen commented 8 years ago

MR uses an event bus. In retrospect, it was not the right design (or something easier should have been used), but it is currently the way it is. The wiki might be slightly out of date but IIRC it covers some of these basics, no?

You need to register an event handler for those events to send the key. That's the short of it.

AnMVCCoder commented 8 years ago

Honestly, I am learning advanced computer programming, especially MVC development, I'm sure I will get to the point where I fully understand advanced concepts like Event Handling - and how to implement it in such a way that would be able to do exactly what you're referring to - with just what you said as an explanation, and not need to come and ask such a follow-up question, and just be able to do it, no problem. But, unfortunately, I'm not at that level currently. I'm honestly sorry I'm asking you for help in such a way, seeing as how you're expecting me to just know. I'm actually embarrassed that I'm asking this.

Please overlook my non-understanding on this, and offer me information on how do I actually set up an event handler that detects the AccountCreatedEvent and reveals the VerificationKey that is associated with an actual account upon account creation - enabling that key to be put in a variable that can be used in the body of an email message to be sent. I really tried to look into your wiki, and the SingleTenant Project, and tried my best to understand how to do this on my own, but I just couldn't figure out a way to do it.

I ended up with this problem because initially (as I previously mentioned) - I had problems sending the verification email itself, originally, I gave the email information in the system.net section of the web.config, and just assumed your system auto-sends the verification email, and found it wasn't just auto-sent, but didn't mind, because a concern of mine was having the control of creating custom verification email messages - so I can phrase it however I wanted, and include custom html and such. But now I see it's harder than I thought to include the proper VerificationKey, so I need this further help I asked from you in the previous paragraph.

Another idea that came to me while writing this was - MembershipReboot is clearly a powerful library offering a much better level of security than the Microsoft defaults, but, and this wasn't obvious to me at the start, it is clearly made for more advanced developers, and those that're like me don't have the knowledge to easily implement it. I hope you can help me in the way I asked in the second paragraph of this post. But, on the other hand, I do remember reading you also offer another library with similar functionality and level of security - as a layer placed on top of the Microsoft defaults - IdentityReboot. I kind of like MembershipReboot even more than IdentityReboot. But a quick question I have is - is IdentityReboot more easy to implement? I mean the Microsoft defaults are pretty simple to work with and there documentation makes it pretty simple to understand. I do understand that you are just one man, and you humbly made this free-to-use, and I truly do appreciate that, and I'm sure everyone else that uses it feels the same. I thank you for creating these libraries, being willing to help people when they have questions, and helping me. I also apologize for having such verbose posts in talking with you. But, on the matter of IdentityReboot, I'd rather use MembershipReboot, and I hope you can help me as I asked in the second paragraph of this post.

brockallen commented 8 years ago

Something like this:

public class CustomEmailEventHandler : IEventHandler<AccountCreatedEvent<UserAccount>>
    {
        public void Handle(AccountCreatedEvent<UserAccount> evt)
        {
            // send evt.VerificationKey to evt.Account.Email
        }
    }

and then register it with the MR config:

var config = new MembershipRebootConfiguration();
config.AddEventHandler(new CustomEmailEventHandler());

That's roughly it. You'd need to implement additional IEventHandler<T> for the other events you cared about from here: https://github.com/brockallen/BrockAllen.MembershipReboot/blob/master/src/BrockAllen.MembershipReboot/AccountService/UserAccountEvents.cs

As for support and which library to use -- you will certainly get more support by using Microsoft's ASP.NET Identity -- many more people use that and thus there are more people to support it (community & Microsoft).

AnMVCCoder commented 8 years ago

Thank you so much, it works! Now that you provided me the details on how to detect the Events, I am almost positive I will have no issues from this point with the system. I might not be as advanced in my understanding of development techniques as I wish, but, I am not exactly an amateur neither, though it may have sounded a bit like it at points in our conversation. Event Handling is something I haven't really done much - especially with custom libraries. But, I do have a pretty good understanding with many c#, ASP.NET, and MVC functionalities. Thanks so much again, issue closed :)

AnMVCCoder commented 8 years ago

I just remembered I wanted to say to you - actually, you're wrong, your event bus system is a good way to do this. I know the way I thought would've been more optimal is pretty standard (like "account.VerificationKey") - but - your event bus way with event handling is good too. And actually, I'm thankful you introduced me to this concept - now I know even more about programming than before - I know event handling, and will obviously use some of these concepts in my own code.

Also, I mentioned in my original question - that I asked the same question on stackoverflow. Obviously it was you who answered my question here, and I thank you. But, since that question is there on stackoverflow, it's best to have the question answered there too. So, I hope you read this, and I'm unsure of how GitHub works, maybe you'll be notified about this comment I just made, and if so, I assume you'll read it. But, without some kind of response from you, I'll be unsure of that. Since you answered my question here, I would hope you can go over to that stackoverflow page with my question and answer it there (with the answer that worked here), and I would mark that as the answer. I'll give that some time, and if you don't answer that question there for me to mark it as the answer, or respond to this comment in the next 5 days - then I'll just answer my question with the answer that worked here on stackoverflow on that page I linked to. Thanks a lot again.

brockallen commented 8 years ago

Go ahead and answer it on SO if it's the right thing. No worries, and thanks for following up here.