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

Discord slash command code to grant AWS access #276

Open
wants to merge 24 commits into
base: develop
Choose a base branch
from

Conversation

vikhyat187
Copy link

@vikhyat187 vikhyat187 commented Oct 19, 2024

Date: 19th Oct

Developer Name: Vikhyat


Issue Ticket Number

Real-Dev-Squad/todo-action-items#253

Description

This change is for the discord command to grant AWS access to the developers, we can tag the developer and the group we wanted to add the user in.

Documentation Updated?

  • Yes
  • [ X ] No

Under Feature Flag

  • Yes
  • [ X ] No

Database Changes

  • Yes
  • [ X ] No

Breaking Changes

  • Yes
  • [ X ] No

Development Tested?

  • [ X ] Yes
  • No

Screenshots

image
Here I've ran the discord command and the user and the AWS group has been specified, it checks if the user is part of the AWS account if not, adds the user to the account and then to the AWS IAM group.
image
This is the screen which the user will see on trying to access the AWS link and they should reset their password by using the password sent to their email. (Emails are taken from our Firestore db)

Test Coverage

Screenshot 1 image
Writing the test cases will push the changes in the same PR
## Additional Notes
  1. We will have to create the groups in our AWS account and then store those values in our discord service before registering the command.
  2. We will have to create IAM access and secret keys and store them in the website backend PR - Added AWS config and identity store website-backend#2208

@vikhyat187 vikhyat187 self-assigned this Oct 19, 2024
@@ -15,7 +15,7 @@
"format-fix": "prettier --write .",
"fix": "npm run lint-fix && npm run format-fix",
"ngrok": "ngrok http 8787",
"register": "ts-node-esm src/register.ts"
"register": "npx ts-node src/register.ts"
Copy link
Author

Choose a reason for hiding this comment

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

Fixed this command as this is causing problem in local development.

Comment on lines 39 to 61
if (!response.ok) {
return fetch(`${DISCORD_BASE_URL}/channels/${channelId}/messages`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bot ${env.DISCORD_TOKEN}`,
},
body: JSON.stringify({
content: `<@${discordUserId}> Error occurred while granting AWS access: ${response.status} ${response.statusText}`,
}),
});
} else {
return fetch(`${DISCORD_BASE_URL}/channels/${channelId}/messages`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bot ${env.DISCORD_TOKEN}`,
},
body: JSON.stringify({
content: `AWS access granted successfully <@${discordUserId}>! Please head over to AWS - ${AWS_IAM_SIGNIN_URL}.`,
}),
});
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Generate message based on condition and write API call code only once

Copy link
Author

Choose a reason for hiding this comment

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

sure

Copy link
Author

Choose a reason for hiding this comment

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

updated this

Comment on lines 63 to 72
return fetch(`${DISCORD_BASE_URL}/channels/${channelId}/messages`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bot ${env.DISCORD_TOKEN}`,
},
body: JSON.stringify({
content: `[TraceId: ${TraceId}] <@${discordUserId}> Error occurred while granting AWS access.`,
}),
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This also, same comment as above

Copy link
Author

Choose a reason for hiding this comment

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

updated this code, please check

Comment on lines 50 to 53
{
name: "S3 read only access",
value: process.env.S3_READ_ONLY_ACCESS_AWS_GROUP_ID,
},
Copy link
Contributor

Choose a reason for hiding this comment

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

We will never do this, I think

Copy link
Author

Choose a reason for hiding this comment

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

Yeah these have to be replaced with actual values

Comment on lines 54 to 62
{
name: "EC2 deployment access",
value: process.env.EC2_DEPLOYMENT_ACCESS_AWS_GROUP_ID,
},
{
name: "DDB read only access",
value: process.env.DDB_READ_ONLY_ACCESS_AWS_GROUP_ID,
},
],
Copy link
Contributor

Choose a reason for hiding this comment

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

Please add some valid once here

channelId: number
) {
const authToken = await jwt.sign(
{ name: "Cloudflare Worker", exp: Math.floor(Date.now() / 1000) + 2 },
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the name here?

Copy link
Author

@vikhyat187 vikhyat187 Oct 31, 2024

Choose a reason for hiding this comment

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

in the website backend, we check if the auth token has the name "cloudfare workers" to validate the token coming from right source.

Ref : https://github.com/Real-Dev-Squad/website-backend/blob/f8d76e1936647759ef7fdfd513f7ff03de226346/middlewares/authorizeBot.js#L18

Copy link
Contributor

Choose a reason for hiding this comment

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

how is a string helping in validating the right source?

Copy link
Author

Choose a reason for hiding this comment

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

So we create a token in the discord slash commands with name present in payload, this is signed by using the private key and in website backend we validate if the same text is being received post validating the token.
@prakashchoudhary07

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 we need to discuss on this one

Copy link
Author

Choose a reason for hiding this comment

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

TraceId: string,
channelId: number
) {
const authToken = await jwt.sign(
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't we already have this? why write it again?

Copy link
Author

Choose a reason for hiding this comment

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

didn't get this comment, we are using this in similar manner in one of the other API too.

Copy link
Contributor

Choose a reason for hiding this comment

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

So at every place we are manually signing, So don't already have a common thing to do this, if not then please create one?

Copy link
Author

Choose a reason for hiding this comment

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

Have created a common utility for this.

userId: discordUserId,
};

const url = `${base_url}/aws-access/`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you create the URL in constant it self?

Copy link
Contributor

Choose a reason for hiding this comment

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

And please share the doc for using it herE?

Copy link
Author

Choose a reason for hiding this comment

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

do we mean it like
const url = ${base_url}/aws-access this way we have it stored?

Copy link
Author

Choose a reason for hiding this comment

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

Didn't get this which doc?

And please share the doc for using it herE?

Copy link
Contributor

Choose a reason for hiding this comment

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

Please share the AWS API docs like, of what they are doing?

Copy link
Author

Choose a reason for hiding this comment

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

This is our website backend API, will add the comments there.

Comment on lines 45 to 68
await fetch(`${DISCORD_BASE_URL}/channels/${channelId}/messages`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bot ${env.DISCORD_TOKEN}`,
},
body: JSON.stringify({
content: content,
}),
});
} catch (err) {
const content = `<@${discordUserId}> Error occurred while granting AWS access.`;
await fetch(`${DISCORD_BASE_URL}/channels/${channelId}/messages`, {
method: "POST",
headers: {
"Content-Type": "application/json",
Authorization: `Bot ${env.DISCORD_TOKEN}`,
},
body: JSON.stringify({
content: content,
}),
});
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

RE: as same, please refactor this. Also please maintain a separate constant for URL's

Copy link
Author

Choose a reason for hiding this comment

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

done updated

userId: discordUserId,
};

const url = `${base_url}/aws-access`;
Copy link
Contributor

Choose a reason for hiding this comment

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

Please move the URL to a separate constant sir

Copy link
Contributor

Choose a reason for hiding this comment

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

Also please name it more appropriately

Copy link
Author

Choose a reason for hiding this comment

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

Have created as a constant in the same file as we can use the base URL depending on the env as per config

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.

3 participants