ericvaandering / DocDB

Working repository for DocDB
25 stars 20 forks source link

Need all instances links for access e-mails #58

Closed ericvaandering closed 6 years ago

ericvaandering commented 6 years ago

In "John Doe requests access to XYZ DocDB" email: The instance admin is given a link only to the DocDB version the user was accessing, as opposed to showing links for all the DocDB versions with variables defined in ProjectGlobals Preferences{Security}{Instances}{FNALSSO/Public/Basic/Certificate}

ericvaandering commented 6 years ago

Fixed by #25 FNAL_SSO branch

lauramengel commented 6 years ago

The email was changed, however the URLs for SSO/Cert/Basic were empty.

Next is what the access request email looked like and approx how we request it look: (added some text to explain transfer option and some text if the user requested a group they should not be allowed, if you're opening this up anyway)

The access request email to admins looked like:

John Smith (Certificate DN /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smith/CN=UID:jsmith) with e-mail address jsmith@fnal.gov has requested access to ESHQ DocDB DocDB. He or she requests membership in the following groups: esh1 esh2

If this is correct, please visit one of these URLs: Services account / Single Sign-On URL: Certificate URL: DocDB username/password URL: Select "Modify", select the user, check "Verify", and click to Submit.

If the groups are not correct, select the correct groups before clicking Submit.

The user attached this note to their application: I will do the verification step.

approx how we request it to look:

John Smith (Certificate DN /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smith/CN=UID:jsmith) with e-mail address jsmith@fnal.gov has requested access to ESHQ DocDB DocDB. He or she requests membership in the following groups: esh1 esh2

The user attached this note to their application: I will do the verification step.

If this is correct, please visit one of these URLs:

Services account / Single Sign-On URL: https://hostname.fnal.gov/cgi-bin/sso/EmailAdministerForm

Certificate URL: https://hostname.fnal.gov/cgi-bin/cert/EmailAdministerForm

DocDB username/password URL: https://hostname.fnal.gov/cgi-bin/private/EmailAdministerForm

If you want to give the new account the same permissions as a user's existing account: Select "Transfer", select the user's old account from which to transfer permissions in the top scrolled list, select the user's new account to which to transfer permissions in the bottom scrolled list, and click "Modify Personal Account".

If you want to authorize the new account without transferring from an old account: If the groups are not correct, select "Clear User's Groups" and then select all the correct groups before clicking "Modify Personal Account". Select "Modify", select the user, check "Verify", and click "Modify Personal Account".

ericvaandering commented 6 years ago

Should be fixed in https://github.com/ericvaandering/DocDB/commit/99368a973196abc8f4ae6f8e95a1cb9260e6db26

I changed the wording a bit, most notably reversing the two clauses in the final paragraph.

lauramengel commented 6 years ago

The links are added now based on the ProjectGlobals variables, as desired

Ok about the order reversal in the modify paragraph, since that puts the most commonly needed instructions first. If someone does not read ahead then they've already modified the account before they see the instr to clear groups. But they can always redo it in that case.

The transfer paragraph was omitted, only the "modify" paragraph was in the email. I'm ok with the transfer paragraph being second after the modify paragraph if you prefer. Text:

If you want to give the new account the same permissions as a user's existing account: Select "Transfer", select the user's old account from which to transfer permissions in the top scrolled list, select the user's new account to which to transfer permissions in the bottom scrolled list, and click "Modify Personal Account".

ericvaandering commented 6 years ago

I'm not sure what you mean by the transfer paragraph being omitted. It is omitted on the e-mail sent out by the certificate version, but not the email sent out by the SSO version. I can add it to both. Is that what you want?

I assumed we were talking about the SSO version. Any changes to the certificate version were just to harmonize things.

I don't have any problem with the ordering of the two paragraphs, but can change it if you prefer.

lauramengel commented 6 years ago

Hi. Yes, thanks - we'd like the transfer paragraph added for both requests for access.

The transfer paragraph can be first or second. If you put it first, then you probably want to add "Otherwise," before "select "Modify", select the user, check "Verify"...

ericvaandering commented 6 years ago

To keep things simple, I left the ordering as is, added Otherwise and made both bits of code say the same thing. Please run one last check to make sure all the words come out, punctuation, etc.

lauramengel commented 6 years ago

Hi. The message looks good, but we have one issue. Regardless of whether the admin is using cert, sso, or password version to manage the accounts: If the user is using the SSO version and requested more groups for their (already verified) SSO account, the admin has to select the requested groups because they are not pre-added.

It is correct that groups are not pre-added, because then a verified user would get the permissions until the admin vetoed it.

However, I think we need to let the admins know in the message that they need to select the requested groups (since they don't have to select them in the other case, which they have gotten used to).

So case A) Access request came from cert version of DocDB

Case B) Access request came from SSO version of DocDB

Otherwise, if you want to authorize the new account without transferring from an old account: Select "Modify", select the user, select each group the user requested (if the groups are correct), check "Verify", and click "Modify Personal Account".

The "clear groups" action is not needed in this case because the groups were not preloaded.

It would also be good to possibly change the subject and first sentence, so the admin can tell the difference between an access request where they need to select the groups vs the request where the groups are pre-loaded for them.

ericvaandering commented 6 years ago

It would also be good to possibly change the subject and first sentence, so the admin can tell the difference between an access request where they need to select the groups vs the request where the groups are pre-loaded for them.

Please give me the exact wording you'd like. This is getting far enough into operations that you know much better than me.

lauramengel commented 6 years ago

Ok. Here's a proposed wording. "Case 1" is no change. For "Case 2", there is 1 change in the subject and 6 changes in the text. I put the changes between starting/ending double asterisks so they're easier to find.

Case 1) E-mail for access request for unverified account:

Keep current message as-is.

Case 2) Additional groups request for already verified SSO or certificate account:

Subject: \<name> requests membership in more groups for access to \<which> DocDB

\<name> (username/Certificate DN \<account>) with e-mail address \<email> has requested membership in more groups for access to \<which> DocDB. He or she requests membership in the following additional groups:

The user attached this note to their application: \<note>

If this is correct, please visit one of these URLs:

Note: You must select the appropriate requested groups in the groups scrolled list to add the user to these additional groups.

Services account / Single Sign-On URL: \<SSO URL>

Certificate URL: \<CERT URL>

DocDB username/password URL: \<Basic Auth URL>

If you want to give the new account the same permissions as a user's existing account: Select "Transfer", select the user's old account from which to transfer permissions in the top list, select the user's new account to which to transfer permissions in the bottom list, and click "Modify Personal Account".

Otherwise, if you want to add groups to the account without transferring from an old account: Select "Modify", select the user, select the appropriate additional groups, check "Verify", and click "Modify Personal Account".

I removed text about "Clear Groups" which is not needed since groups were not pre-loaded in this case

lauramengel commented 6 years ago

To make the form to apply for more groups more discoverable, what would you think of adding a link in the left box of the DocumentDatabase page for SSO and CERT labeled something like "Apply for More Groups" (between "Preferences" and "Limit Groups" links) that goes to your existing form to request more groups for an already verified account. It could be added for just SSO and not CERT at this point, if you prefer.

ericvaandering commented 6 years ago

This would be a decent sized modification of the form to not make any DB level changes but just send e-mail, right?

I'd rather push this off until you have some experience with the SSO version in the wild and we know how many groups are being managed through ADFS and how many directly in DocDB.

I'll work on the wording now.

ericvaandering commented 6 years ago

You are right that it doesn't require any code changes, I think. I changed the wording and I put a link in the side bar for the SSO version. I can't do it for the cert version because there the application is a one time only thing. It would require a re-write of UserAccessApply

ericvaandering commented 6 years ago

So try again after an update.

ericvaandering commented 6 years ago

Take a look at how "Apply for More Groups" looks. If it's crowded we can do "Apply for Groups" or "Apply to Groups"

lauramengel commented 6 years ago

I fixed my formatting in the proposed wording post because all my text in less than and greater than symbols such as \<usernamehere> had disappeared.

lauramengel commented 6 years ago

I counted before and it's only 5 chars more than the longest one already there.

However, looking at it now, it sure does stick out.

So your shorter suggestion will be better: Apply for Groups

ericvaandering commented 6 years ago

Message updated

lauramengel commented 6 years ago

Ran into one problem and one wording request.

Will post problem next.

lauramengel commented 6 years ago

When I requested access for a new certificate (as a user), and then went to grant that access using the SSO link in the email, the results screen had all blank values and the user's new certificate was not verified. Please see the following 5 screengrabs: (If they don't show in order, the one with "1" in the filename is first and "5" last.

docdb_blankresult_verifyuser1 docdb_blankresult_userverify2 docdb_blankresult_verifyuser3 docdb_blankresult_verifyuser4 docdb_blankresult_verifyuser5
lauramengel commented 6 years ago

The SSO add groups worked well. (Did the expected action and the results screen did not have blanks.

ericvaandering commented 6 years ago

I cannot spot the problem in verifying. I added some extra debug messages. Can you rerun this but give me the debug messages from the bottom of EmailAdminister. Feel free to snip out any sensitive fields. Somehow it looks like a variable is getting lost.

lauramengel commented 6 years ago

No. The verification did not work.

lauramengel commented 6 years ago

Sigh. It worked perfectly this time.

Did you change anything besides the debugging statements? If I hadn't saved the screen dumps, I wouldn't believe me.

Do you want to take the debugging statements out and I'll try again?

Here is what the debugging statements said at each step.

on https://esh-docdbdevcert.fnal.gov/cgi-bin/cert/CertificateApplyForm before applying:

Debugging messages: At DocDBFooter Finding status by DN /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smith/CN=UID:jsmith Certificate Status: noapp DN is /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smithl/CN=UID:jsmith, CN is John Smith Getting all security groups Finding EmailUserID and groups by DN /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smith/CN=UID:jsmith Found e-mail user: User is not verified Before limiting, user belongs to groups After limiting, user belongs to unique groups User belongs to groups User can create: 0

on https://esh-docdbdevcert.fnal.gov/cgi-bin/cert/UserAccessApply after (results) of applying

Debugging messages: At DocDBFooter Finding status by DN /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smith/CN=UID:jsmith Certificate Status: noapp Finding EmailUserID and groups by DN /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=John Smith/CN=UID:jsmith Found e-mail user: User is not verified Before limiting, user belongs to groups After limiting, user belongs to unique groups User belongs to groups User can create: 0

on https://esh-docdbdev.fnal.gov/cgi-bin/sso//EmailAdministerForm before verifying

Debugging messages: At DocDBFooter Getting all security groups Finding EmailUserID by FNAL SSO name lauram@fnal.gov Determined user ID to be 993 User explicity has groups 1 3 After SSO groups, DocDB groups for user: 1, 3, 6 Final unique DocDB groups for user: 6, 1, 3 Before limiting, user belongs to groups 6, 1, 3 After limiting, user belongs to unique groups 6, 1, 3 Finding EmailUserID by FNAL SSO name lauram@fnal.gov Determined user ID to be 993 User explicity has groups 1 3 After SSO groups, DocDB groups for user: 1, 3, 6 Final unique DocDB groups for user: 1, 6, 3 Before limiting, user belongs to groups 1, 6, 3 After limiting, user belongs to unique groups 1, 6, 3 User belongs to groups 1, 6, 3 User can create: 1

On https://esh-docdbdev.fnal.gov/cgi-bin/sso/EmailAdminister (results) after verifying

Debugging messages: At DocDBFooter Getting all security groups Finding EmailUserID by FNAL SSO name lauram@fnal.gov Determined user ID to be 993 User explicity has groups 1 3 After SSO groups, DocDB groups for user: 1, 3, 6 Final unique DocDB groups for user: 6, 1, 3 Before limiting, user belongs to groups 6, 1, 3 After limiting, user belongs to unique groups 6, 1, 3 Changing info for EmailUserID 996 Changing info for EmailUserID /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=Laura Mengel/CN=UID:lauram Tried to verify Sending verification mail to user and admin Clearing all EmailUsers Fetching info for EmailUserID 996 Got info for EmailUserID /DC=org/DC=cilogon/C=US/O=Fermi National Accelerator Laboratory/OU=People/CN=Laura Mengel/CN=UID:lauram Finding EmailUserID by FNAL SSO name lauram@fnal.gov Determined user ID to be 993 User explicity has groups 1 3 After SSO groups, DocDB groups for user: 1, 3, 6 Final unique DocDB groups for user: 6, 1, 3 Before limiting, user belongs to groups 6, 1, 3 After limiting, user belongs to unique groups 6, 1, 3 User belongs to groups 6, 1, 3 User can create: 1

ericvaandering commented 6 years ago

Here's what I added: https://github.com/ericvaandering/DocDB/commit/ab11ad21b9084f517bfe7bf8ae001060dbd7ac13

Debug, a spacing change, and changing a feedback string.....

I can't imagine what the difference would be. 993 is the ID of the account you were using. 996 is the ID of the account you were changing. Any chance those were the same in your earlier trial? I don't know what that would do, it's all I can think of.

ericvaandering commented 6 years ago

Removing the debug statements won't change anything. Besides, most of them might be useful. I may remove on or two from the final version.

lauramengel commented 6 years ago

The SSO account might have been the same ID, but the cert ID would be different because I have to delete it each time so I can apply again.

I do not thing the SSO ID and Cert ID were the same as each other. Looks like I was using 935 for SSO and 990 for the cert.

lauramengel commented 6 years ago

I'll try it a few more times and let you know if I am able to reproduce it.

ericvaandering commented 6 years ago

Sounds good. The messages were exactly as I expected. If you trip it, the messages may help

lauramengel commented 6 years ago

I believe this was operator error on my part. Sorry for the false-alarm. The rest of the tests were successful and the 4 messages have the desired wording. Closing issue.

I had a lot of tabs open when I was testing. I think I accidentally used an old EmailAdminister tab and selected an account entry that had later been deleted, when I was modifying the account. This caused the blanks on the result screen and none of the changes to occur on the requested account. I was able to reproduce this by leaving up and old EmailAdminister form with a deleted account. I was not able to reproduce when I used a refreshed EmailAdminister form with the latest information.