Skip to content
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

Fixes #3168 Test Sending Email Issues #3237

Merged
merged 7 commits into from
Nov 13, 2019

Conversation

thabaum
Copy link
Contributor

@thabaum thabaum commented Nov 4, 2019

Fixes
#3168
https://www.dnnsoftware.com/forums/threadid/86505/scope/posts/change-portal-administrator-email-address

Summary

I may get shot down here but it is worth a try. Thanks for the dnndocs.com it helped me find some things I was looking for. I found a typo there I will submit a fix for as well.

Please help me if I went wrong here it just didnt make sense the way it was written when setting the To and From addresses nothing referenced the portal administrator email. And the send to address it feels to me like it should be going to whoever is logged in testing it so they can see it works.

I also am not sure if this actually uses the email being set in Host SMTP Settings. For testing the emails this is where it needs set so any changes to be made...

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.
Copy link
Contributor

@david-poindexter david-poindexter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thabaum please rebase this PR on the current release/9.4.x branch. Right now you have 44 commits in this PR (many from other PRs). Thanks 🙏🏼

@bdukes
Copy link
Contributor

bdukes commented Nov 4, 2019

@thabaum I've rebased your branch on release/9.4.x

@thabaum
Copy link
Contributor Author

thabaum commented Nov 4, 2019

sorry, trying to figure this out here not trying to get put anywhere special I need to understand this a bit more please direct, that is strike 3 however, that is my next goal... figure out what I am doing wrong with the PR's I think I need more coffee, time for a brake :) I will get back to work on this. I want to get one right. I will hold off on my next one until I re-review all the latest tutorials.

Now when you say Rebase, can you link me to how

@thabaum I've rebased your branch on release/9.4.x

ok now how did you do that :) I thought I had it on that branch when I created my PR... did something change?

@thabaum
Copy link
Contributor Author

thabaum commented Nov 4, 2019

OK i see the changes no commits now.

@bdukes
Copy link
Contributor

bdukes commented Nov 4, 2019

When you make your changes, you want to start on the release/9.4.x branch (i.e. that would be the starting point of your new branch for the PR). Since you started on development, I did something like git rebase origin/release/9.4.x --interactive, which replays all of the commits from your branch onto release/9.4.x. It does this interactively, meaning that it opens up a text file with a list of the commits it is going to replay, and allows you to make adjustments to it. In this case, I adjusted that list by deleting all but your three commits.

@bdukes
Copy link
Contributor

bdukes commented Nov 4, 2019

Build error:

Services\ServerSettingsSmtpAdminController.cs(123,64): error CS0103: The name Portals does not exist in the current context [D:\a\1\s\Dnn.AdminExperience\Extensions\Content\Dnn.PersonaBar.Extensions\Dnn.PersonaBar.Extensions.csproj]

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?
I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.
@thabaum
Copy link
Contributor Author

thabaum commented Nov 5, 2019

I think I have addressed all concerns here.

If all is correct the Send From email address if smtpHostMode is TRUE then host email and if false portal Primary Administrator email is used located here in Settings => Security:

image

The Send To address will be the users email address that is currently used to run the test. You can then login using a second administrator account and switch your administrator email address to whatever you like and test emails going to msn, yahoo, google... with ease I believe if this works as planned.

If I am getting warmer but still need to make a change to make this work right please let me know and thanks in advance and for the past help understanding this.

@thabaum
Copy link
Contributor Author

thabaum commented Nov 5, 2019

When you make your changes, you want to start on the release/9.4.x branch (i.e. that would be the starting point of your new branch for the PR). Since you started on development, I did something like git rebase origin/release/9.4.x --interactive, which replays all of the commits from your branch onto release/9.4.x. It does this interactively, meaning that it opens up a text file with a list of the commits it is going to replay, and allows you to make adjustments to it. In this case, I adjusted that list by deleting all but your three commits.

@thabaum please rebase this PR on the current release/9.4.x branch. Right now you have 44 commits in this PR (many from other PRs). Thanks 🙏🏼

I believe I know where I am going wrong as I need to start out with the branch I am working on, it's never that easy is it :). I will hopefully get it down for the next PR sorry it is taking me a few times to really understand what I am doing here. Thank you for the patience and support.

var mailFrom = Host.HostEmail;
var mailTo = smtpHostMode ? Host.HostEmail : PortalSettings.UserInfo.Email;
var mailFrom = smtpHostMode ? Host.HostEmail : PortalSettings.Email;
var mailTo = PortalSettings.Current.UserInfo.Email;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is sending the email to the administrator user, weren't you trying to have it send to the current user? That would be UserController.Instance.GetCurrentUserInfo().

Copy link
Contributor Author

@thabaum thabaum Nov 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PortalSettings.Current.UserInfo.Email
change to
UserController.Instance.GetCurrentUserInfo("Email")

and this should get the current user email?

This is what I am looking for doing and not the portal administrator. This way you can send emails pretty much anywhere to test by logging in with different host or admin accounts, not just to the same domain admin account.

I went ahead changed it to UserController.Instance.GetCurrentUserInfo("Email") to get the current user email. Is there a need to add using DotNetNuke.Entities.Users to reference that namespace or will it work as is.

Or can I get the current user by simply putting UserInfo.Email ?

image

What is the difference between these?
PortalSettings.Email
PortalSettings.UserInfo.Email
PortalSettings.Current.UserInfo.Email

And which would be best practice to use for getting the administrators email account?

@thabaum thabaum requested a review from bdukes November 6, 2019 04:20
Copy link
Contributor

@bdukes bdukes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@thabaum
Copy link
Contributor Author

thabaum commented Nov 6, 2019

A few issues should reference:

#2298
#2227
#2298
#3168
and the ones dating back to DNN version 4 like this one in 2006
https://www.dnnsoftware.com/forums/threadid/86505/scope/posts/change-portal-administrator-email-address

I will now verify these portal settings FROM address are in fact used for actually sending the email notifications. This will be a separate PR if I find anything that can be tweaked.

It feels like if I introduce a section of an issue and not combine a huge issue it is easier to chew on. Similar to another issue created that I want to be able to edit site settings without having to log into each portal. Take on the entire project I think is a lot for one person to do, however take on one page/PR at a time and it will get done without feeling so overwhelmed.

I am also interested in trying to work on some bigger issues with a few takers that can help collaborate on an issue together. I just need some help getting started with a few things.

I want to add two way verification, SMS and another security feature to login along with fix any registration issues as needed so the entire system that has to do with user communication and account security is 100% by version 10.

This is my own personal goal if anyone is taking on these tasks please let me know so I can push on some other direction. You pretty much own me until these are done as I am highly motivated right now and allocating resources (which are unfortunately not much currently) to make it happen.

And I greatly appreciate all of your help, it is huge to get a good understanding of how to use all my so called book smarts as I really never truly understand anything until I actually get my hands dirty. And a hello world app never really did it for me. A lot of refresher stuff going on until I get more fluent.

@thabaum
Copy link
Contributor Author

thabaum commented Nov 11, 2019

@david-poindexter Everything good here?

@mitchelsellers mitchelsellers added this to the 9.4.3 milestone Nov 13, 2019
@mitchelsellers mitchelsellers merged commit ea34bfa into dnnsoftware:release/9.4.x Nov 13, 2019
donker pushed a commit that referenced this pull request Nov 15, 2019
* Do not store these portal settings when setting their property on the portalsettings object

* Removed DnnHttpControllerActivator and added DependencyResolver implementation using the IServiceProvider

* Added xml comments for the new DnnDependencyResolver

* Added DnnDependencyResolver Unit Tests

* Replaced IServiceProvider mock with full IServiceProvider implementation to work with Web API tests

* Added more unit tests for the DnnDependencyResolver to cover GetServices (multiple) and BeginScope

* Do not show "No Search results" message when no search is performed. (#3203)

* Revision of build process part 1 (#3137)

* Move website folder

* repairs

* Update Cake

* Move external task to its own file

* Begin packaging scripts in cake

* Further work on packaging

* Ensure Telerik project also gets new version nr

* Move Telerik PDF to its proper place

* New logic for packages that are already in final form in the repo and just need to be zipped.

* Bring fips Lucene lib back under git control

* Newtonsoft package and repair to telerik package

* Splitting off other tasks

* Upgrade and deploy packages

* Symbols package

* Get rid of platform ver in manifest

* Final fixes - working packages

* Move compilation stuff

* House cleaning

* peg utils version

* Fix to ckep script

* suppress warnings

* Variable initialization issue

* Removing unused scripts

* Revert change to Telerik package version

* Move "other" to "thirdparty"

* Get rid of hard coded Newtonsoft version

* Max cpu to 4 for compiling

* Repairs to MSBuild scripts to get AE to build properly

* Correcting paths

* Cleaning up

* Trying to fix ignore file

* Update Build/Cake/thirdparty.json

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Remove absolute path from .gitignore

* Fix extension of DNC library package

* Merging AE and DNN build scripts

* Fixes to scripts

* Clean up symbols project for AE since they are included in DNN project now

* Add previously untracked Yahoo compressor dlls

* Remove version information from SolutionInfo.cs

* Fix errors introduced in rebase

* Fix incorrect paths

* Change build order of MSBuild to build extensions before personabar ui.

* Make white-space visible in log viewer (#3198)

* DNN-34236: check the url with case insensitive.

* DNN-29417 Username is changed to email address in the user profile if the "Use Email Address as Username" is enabled.

* Fix to installer and the dependencies of the HTML and DA modules

* DNN-24945 - Always redirect to the source portal page after creating (#3223)

group

* Fix Typo (#3234)

* Improve querystring parse (#3242)

* Improve props popup

* Improve input checking of sites (#3243)

* Fixes an upgrade issue that affects 9.4.1 => 9.4.2 upgrades (#3282)

* Fixes an issue with a duplication of assembly binding

* Removed hardcode version in manifest

* Further reorganization of build process (#3236)

* Introduce settings and add custom version nr

* Ability to create a local dev site

* Move node workspaces to root

* Reshuffling of variables in MSBuild

* Repairs

* Fix

* Allow local override of global build variables

* Only save manifests if the file is not there. This prevents overwriting in case of a failed build before.

* Ensure projects build in debug to the website path specified by the platform build settings

* Don't track vscode folder

* Fixes #3168 Test Sending Email Issues (#3237)

* Use Admin Email for Send if smtpHostMode false

* Fix Test Email Settings TO and FROM Addresses

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.

* Update ServerSettingsSmtpAdminController.cs

* Fix build issue to previous changes.

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?

* Change to address to the current user's email testing the server.

I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.

* Corrected To address for current user email

* TO Current User Email

* DNN-29110 Site Assets > Select All is not working (#3251)

* DNN-29110 Site Assets > Select All is not working

* Code review fix

* Moves Remember Login above Login Button:  referenced in issue #2471 #3255 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...

* DNN-34250 Search is not working even after re-Indexing Search for All DNN Portals (#3260)

* Fixes duplication in DotNetNuke.Website.csproj

* Removed reference to missing LeftMenu files
valadas pushed a commit that referenced this pull request Dec 3, 2019
* Do not store these portal settings when setting their property on the portalsettings object

* Removed DnnHttpControllerActivator and added DependencyResolver implementation using the IServiceProvider

* Added xml comments for the new DnnDependencyResolver

* Added DnnDependencyResolver Unit Tests

* Replaced IServiceProvider mock with full IServiceProvider implementation to work with Web API tests

* Added more unit tests for the DnnDependencyResolver to cover GetServices (multiple) and BeginScope

* Do not show "No Search results" message when no search is performed. (#3203)

* Revision of build process part 1 (#3137)

* Move website folder

* repairs

* Update Cake

* Move external task to its own file

* Begin packaging scripts in cake

* Further work on packaging

* Ensure Telerik project also gets new version nr

* Move Telerik PDF to its proper place

* New logic for packages that are already in final form in the repo and just need to be zipped.

* Bring fips Lucene lib back under git control

* Newtonsoft package and repair to telerik package

* Splitting off other tasks

* Upgrade and deploy packages

* Symbols package

* Get rid of platform ver in manifest

* Final fixes - working packages

* Move compilation stuff

* House cleaning

* peg utils version

* Fix to ckep script

* suppress warnings

* Variable initialization issue

* Removing unused scripts

* Revert change to Telerik package version

* Move "other" to "thirdparty"

* Get rid of hard coded Newtonsoft version

* Max cpu to 4 for compiling

* Repairs to MSBuild scripts to get AE to build properly

* Correcting paths

* Cleaning up

* Trying to fix ignore file

* Update Build/Cake/thirdparty.json

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Remove absolute path from .gitignore

* Fix extension of DNC library package

* Merging AE and DNN build scripts

* Fixes to scripts

* Clean up symbols project for AE since they are included in DNN project now

* Add previously untracked Yahoo compressor dlls

* Remove version information from SolutionInfo.cs

* Fix errors introduced in rebase

* Fix incorrect paths

* Change build order of MSBuild to build extensions before personabar ui.

* Make white-space visible in log viewer (#3198)

* DNN-34236: check the url with case insensitive.

* DNN-29417 Username is changed to email address in the user profile if the "Use Email Address as Username" is enabled.

* Fix to installer and the dependencies of the HTML and DA modules

* DNN-24945 - Always redirect to the source portal page after creating (#3223)

group

* Fix Typo (#3234)

* Improve querystring parse (#3242)

* Improve props popup

* Improve input checking of sites (#3243)

* Fixes an upgrade issue that affects 9.4.1 => 9.4.2 upgrades (#3282)

* Fixes an issue with a duplication of assembly binding

* Removed hardcode version in manifest

* Further reorganization of build process (#3236)

* Introduce settings and add custom version nr

* Ability to create a local dev site

* Move node workspaces to root

* Reshuffling of variables in MSBuild

* Repairs

* Fix

* Allow local override of global build variables

* Only save manifests if the file is not there. This prevents overwriting in case of a failed build before.

* Ensure projects build in debug to the website path specified by the platform build settings

* Don't track vscode folder

* Fixes #3168 Test Sending Email Issues (#3237)

* Use Admin Email for Send if smtpHostMode false

* Fix Test Email Settings TO and FROM Addresses

If I did this right and it should make sense is I set the test email to be able to go to the user testing.  After all they are who are trying to make it work.  It can be any email used by anyone and usually to see this feature you are a host or portal administrator so security should not be as much of a concern here.

The FROM address will use the HOST email settings if set to HOST mode in DNN for the portal, otherwise it should use the Portal Administrators Email account to send from.

This however does not address the issue of using the email address supplied in settings and creating another setting which will be applied next to figure out the correct logic.

* Update ServerSettingsSmtpAdminController.cs

* Fix build issue to previous changes.

PortalSettings.UserInfo.Email 
is this the current user email?

I am still getting my things setup.  I ran VS 2019 to build successfully and I am trying to read through the namespaces correctly so I can understand the changes.  I noticed a number of files possibly needing updates due to being deprecated in 9.4.2 but after I changed the namespace issue those warnings went away.  

One last thought is how do you get intellisense working on DNN solution.  Is that a dream to have or am I missing something in my setup?

* Change to address to the current user's email testing the server.

I believe a UI to set the TO address would be nice to help troubleshoot and address issues trying to send to a specific user.

* Corrected To address for current user email

* TO Current User Email

* DNN-29110 Site Assets > Select All is not working (#3251)

* DNN-29110 Site Assets > Select All is not working

* Code review fix

* Moves Remember Login above Login Button:  referenced in issue #2471 #3255 (#3256)

* Move Remember Login Above Login Button

* Update Login.ascx

* file copy paste issue...

* DNN-34250 Search is not working even after re-Indexing Search for All DNN Portals (#3260)

* DNN-32474 - Recursive delete option is added (#3249)

* Move Country Above Region in UserProfile.cs

* spacing issue resolved.

* I keep saving but github is moving it on its own.

* Fixed tab for spaces to resolve spacing issues

* Add PortalSettings overloads back (#3295)

Fixes #3289

* Updates issue templates as per 9.4.2 release

* Move Email Above Username in User.ascx #3305 (#3306)

* Move Email Above Username in User.ascx

* Fixed tab index values in User.ascx

* Fix NuGet warning NU5048 (#3304)

The iconUrl property of the nuspec file is deprecated, it's recommended
to add an icon property, which points to a file within the package,
while also retaining the iconUrl property for older clients.
See https://docs.microsoft.com/en-us/nuget/reference/nuspec#iconurl

* Word-Break added to Journal P (#3307)

* Set core object versions to core version (#3287)

Ensure skin and radeditorprovider get the core version nr and add SPROC to set the version of all core packages/desktopmodules to the core version

Fixes #3239

* Third installment of build reorganization (#3285)

* Improve versioning

* New approach: keep versions in source control and change upon CI build

* Remove previous versioning logic

* Cleaning up tasks

* Fixes

* Ensure UpdateDnnManifests targets just the right manifests

* Harmonize naming of zips and nuget packages

* Testing webpack building to dev website

* Adjust webpack projects in AE to build to dev site

* Fixes

* Delete duplicate stuff

* This is generated by Webpack and shouldn't be tracked

* Include missing project from Lerna

* Fixes for building SitesListView

* Update to packages

* Further cleaning

* Remove RadEditorProvider

* Fix ModuleSettings > Added to Pages grid, pager wasn't working

* Allow only alphanumeric characters to be used when forming url from website name (#3316)

* Ensure UpdateDnnManifests doesn't run for building dev site (#3314)

* Reload page when the code tells you to (#3315)

* Reload page when the code tells you to

* Optimize code

* Google Analytics TrackingID is stored in lowercase #3318 (#3322)

* Update sums.resources

* Fix casing mismatch errors in security analyzer file compare

* Update Dnn.AdminExperience/Extensions/Content/Dnn.PersonaBar.Extensions/Components/Security/Checks/CheckDefaultPage.cs

Co-Authored-By: Brian Dukes <bdukes@engagesoftware.com>

* Registered IPersonaBarContainer with DI (#3329)

* Document approval process and group

* Fix typo

* Relaxed nuget.cake wildcard (#3331)
@DHCuthill
Copy link

This condition of a placeholder default From Address (admin@changeme.invalid) being sent in system emails did not occur in version
DNN PLATFORM v. 09.06.02 (0)
It has come back in the latest version
DNN PLATFORM v. 09.09.00 (0)

The error has returned. This makes the portal unfit for release to my customers.
Any advice on how to deal with this would be welcome.

@DHCuthill
Copy link

DHCuthill commented Mar 8, 2021

I have a solution that will do for me.

  1. Add a new user "Administrator" and ensure there is a valid email address
  2. Add the new user "Administrator" to the Role of "Administrators".
  3. Go to Login Settings/Basic Login Settings/ Primary Administrator
  4. From the drop-down-list select the new user you created (Administrator)
  5. To test the site auto-email function create a test user and check that the email came from the "Administrator" email address

@valadas
Copy link
Contributor

valadas commented Mar 9, 2021

That default email with .invalid domain name was selected by design so it can never be registered by anyone for security reasons, so as part of portal/site setup one needs to replace it by a valid email.

@thabaum thabaum deleted the patch-2 branch December 7, 2021 03:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants