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.

Sign up for GitHub

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

Add support for multiple users in ad template #5994

Merged
merged 3 commits into from
Jan 8, 2024
Merged

Add support for multiple users in ad template #5994

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ddd6ed1
Add suport for multiple users in ad template
hgreebe Jan 5, 2024
255a0ad
Sanitize user_names
hgreebe Jan 7, 2024
e314928
Merge branch 'develop' into develop
hgreebe Jan 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions cloudformation/ad/ad-integration.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,13 @@ Parameters:
MaxLength: 64
AllowedPattern: (?=^.{8,64}$)((?=.*\d)(?=.*[A-Z])(?=.*[a-z])|(?=.*\d)(?=.*[^A-Za-z0-9\s])(?=.*[a-z])|(?=.*[^A-Za-z0-9\s])(?=.*[A-Z])(?=.*[a-z])|(?=.*\d)(?=.*[A-Z])(?=.*[^A-Za-z0-9\s]))^.*
NoEcho: true
UserName:
Description: Cluster user that is created in the Active Directory.
UserNames:
Description: Comma separated cluster users to create in the Active Directory.
Type: String
Default: user000
MinLength: 3
MaxLength: 64
UserPassword:
Description: Cluster user Password.
Description: Cluster user Password for all the users specified in 'Users'.
Type: String
MinLength: 8
MaxLength: 64
Expand Down Expand Up @@ -431,8 +430,13 @@ Resources:
echo "$ADMIN_PW" | adcli create-user -x -U "${Admin}" --domain="${DirectoryDomain}" --display-name=ReadOnlyUser ReadOnlyUser
sleep 0.5
echo "Registering User..."
echo "$ADMIN_PW" | adcli create-user -x -U "${Admin}" --domain="${DirectoryDomain}" --display-name="${UserName}" "${UserName}"

USERNAMES="${UserNames}"
for username in $(echo $USERNAMES | sed "s/,/ /g")
do
echo "Registering user: $username"
echo "$ADMIN_PW" | adcli create-user -x -U "${Admin}" --domain="${DirectoryDomain}" --display-name="$username" "$username"
done

echo "Creating domain certificate..."
PRIVATE_KEY="${DirectoryDomain}.key"
CERTIFICATE="${DirectoryDomain}.crt"
Expand All @@ -451,7 +455,7 @@ Resources:

- { DirectoryDomain: !GetAtt Prep.DomainName,
AdminPassword: !Ref AdminPassword,
UserName: !Ref UserName,
UserNames: !Ref UserNames,
DnsIp1: !GetAtt Prep.DnsIpAddress1,
DnsIp2: !GetAtt Prep.DnsIpAddress2,
DomainCertificateSecretArn: !Ref DomainCertificateSecret,
Expand Down Expand Up @@ -548,7 +552,7 @@ Resources:
instance_id = event['ResourceProperties']['AdminNodeInstanceId']

read_only_password = event['ResourceProperties']['ReadOnlyPassword']
user_name = event['ResourceProperties']['UserName']
user_names = event['ResourceProperties']['UserNames']
user_password = event['ResourceProperties']['UserPassword']
admin_password = event['ResourceProperties']['AdminPassword']
admin = event['ResourceProperties']['Admin']
Expand All @@ -561,7 +565,9 @@ Resources:
response_data['Message'] = 'Resource creation successful!'
physical_resource_id = create_physical_resource_id()
ds.reset_user_password(DirectoryId=directory_id, UserName='ReadOnlyUser', NewPassword=read_only_password)
ds.reset_user_password(DirectoryId=directory_id, UserName=user_name, NewPassword=user_password)
for name in user_names.split(","):
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest to sanitize user_names by trimming it to avoid the risk of having user names with blank spaces.

The reason why I'm suggesting this here in Python code but not in the bash code above is because in the bash code you implicitly sanitized it using sed replacement within bash arrays.

name = name.strip()
Copy link
Contributor

@gmarciani gmarciani Jan 8, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Minor It's ok to strip here, but just for sharing: in this way you strip once per every username. If you strip user_names when defining the loop instead you strip just once for all.

ds.reset_user_password(DirectoryId=directory_id, UserName=name, NewPassword=user_password)
ds.reset_user_password(DirectoryId=directory_id, UserName=admin, NewPassword=admin_password)
ec2.stop_instances(InstanceIds=[instance_id])

Expand All @@ -576,7 +582,7 @@ Resources:
AdminNodeInstanceId: !Ref AdDomainAdminNode
# DirectoryId: !If [CreateAD, !Ref Directory, !Ref Ad ]
DirectoryId: !If [UseMicrosoftAD, !Ref MicrosoftADDirectory, !Ref SimpleADDirectory ]
UserName: !Ref UserName
UserNames: !Ref UserNames
UserPassword: !Ref UserPassword
AdminPassword: !Ref AdminPassword
ReadOnlyPassword: !Ref ReadOnlyPassword
Expand Down
Loading