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

Add RequestProjectRole API #353

Merged
merged 12 commits into from
Mar 6, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
7 changes: 7 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -409,6 +409,13 @@ test-notification:
-d '{"project_id":1001, "notification_id":1001}' \
$(CORE_API_ADDR) core.alert.AlertService.TestNotification

.PHONY: request-authz-notification
request-authz-notification:
$(GRPCURL) \
-plaintext \
-d '{"project_id":1001, "project_name": "projectA", "user_name":"userA"}' \
$(CORE_API_ADDR) core.alert.AlertService.RequestAuthzNotification

.PHONY: analyze-alert
analyze-alert:
$(GRPCURL) \
Expand Down
29 changes: 29 additions & 0 deletions pkg/server/alert/alert_notification.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"sort"
"time"

"github.com/ca-risken/core/pkg/model"
Expand Down Expand Up @@ -219,6 +220,34 @@ func (a *AlertService) TestNotification(ctx context.Context, req *alert.TestNoti
return &empty.Empty{}, nil
}

func (a *AlertService) RequestAuthzNotification(ctx context.Context, req *alert.RequestAuthzNotificationRequest) (*empty.Empty, error) {
if err := req.Validate(); err != nil {
return nil, err
}
notifications, err := a.repository.ListNotification(ctx, req.ProjectId, "slack", 0, time.Now().Unix())
gassara-kys marked this conversation as resolved.
Show resolved Hide resolved
sort.Slice((*notifications), func(i, j int) bool {
return (*notifications)[i].NotificationID < (*notifications)[j].NotificationID
})
notification := &(*notifications)[0]
gassara-kys marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
Copy link
Contributor Author

@senk8 senk8 Mar 1, 2024

Choose a reason for hiding this comment

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

ここの書き方は他の部分では

	if err != nil {
		if errors.Is(err, gorm.ErrRecordNotFound) {
			return &empty.Empty{}, nil
		}
		return nil, err
	}

となっていますが、

  • このAPIは管理者に権限リクエストを送るもので、何らかのバグで送ることができなかった場合にはエラーをそのまま返してしまうのがよいと考えた

ので、errをそのまま返却するようにしました。

if errors.Is(err, gorm.ErrRecordNotFound) {
return &empty.Empty{}, nil
}
return nil, err
}
switch notification.Type {
case "slack":
err = a.sendSlackRequestAuthzNotification(ctx, a.baseURL, notification.NotifySetting, a.defaultLocale, req.UserName, req.ProjectName, req.ProjectId)
if err != nil {
a.logger.Errorf(ctx, "Error occured when sending request authz slack notification. err: %v", err)
return nil, err
}
default:
a.logger.Warnf(ctx, "This notification_type is unimprement. type: %v", notification.Type)
Copy link
Collaborator

Choose a reason for hiding this comment

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

エラーを返しても良いかもです。
通知されてないのに、正常系のレスポンスになってしまいそうなので

Copy link
Contributor

Choose a reason for hiding this comment

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

すごい細かいですが、unimprement -> unimplement のtypoがありそうです。
ここで使うならunimplementedの方が適切かもと思いました。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

通知されてないのに、正常系のレスポンスになってしまいそうなので
すごい細かいですが、unimprement -> unimplement のtypoがありそうです。ここで使うならunimplementedの方が適切かもと思いました。

こちらについては自分も同感なので、修正します。
また、こちらはTestNotificationのほぼコピペとなっていたので、TestNotificationにも同様の問題(タイポ、エラーを返していないなど)がありそうです。意図していない実装になっている場合、このPRで一緒に修正してしまいます。

func (a *AlertService) TestNotification(ctx context.Context, req *alert.TestNotificationRequest) (*empty.Empty, error) {

}
return &empty.Empty{}, nil
}

func (a *AlertService) convertNotification(ctx context.Context, n *model.Notification, mask bool) (*alert.Notification, error) {
if n == nil {
return &alert.Notification{}, nil
Expand Down
66 changes: 62 additions & 4 deletions pkg/server/alert/alert_slack.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,15 @@ const (
- Remove the root cause of the problem
- If it is an intentional setup/operation and the risk is small, archive it
- If the nature of the problem is not urgent and immediate action is difficult, set a target deadline and PEND`
slackNotificationAttachmentJa = "その他、%d件すべてのFindingは <%s/alert/alert?project_id=%d&from=slack|アラート画面> からご確認ください。"
slackNotificationAttachmentEn = "Please check all %d Findings from <%s/alert/alert?project_id=%d&from=slack|Alert screen>."
slackNotificationTestMessageJa = "RISKENからのテスト通知です"
slackNotificationTestMessageEn = "This is a test notification from RISKEN"
slackNotificationAttachmentJa = "その他、%d件すべてのFindingは <%s/alert/alert?project_id=%d&from=slack|アラート画面> からご確認ください。"
slackNotificationAttachmentEn = "Please check all %d Findings from <%s/alert/alert?project_id=%d&from=slack|Alert screen>."
slackNotificationTestMessageJa = "RISKENからのテスト通知です"
slackNotificationTestMessageEn = "This is a test notification from RISKEN"
slackRequestAuthzNotificationMessageJa = `<!here> %sさんが
gassara-kys marked this conversation as resolved.
Show resolved Hide resolved
あなたのプロジェクト%sへのアクセスをリクエストしました。
問題がなければ<%s/iam/user?project_id=%d&from=slack|ユーザー一覧>から%sさんを招待してください。`
slackRequestAuthzNotificationMessageEn = `<!here> %s has requested access to your Project %s.
If there are no issues, please check <%s/iam/user?project_id=%d&from=slack|the user list> and invite Person %s.`
)

func (a *AlertService) sendSlackNotification(
Expand Down Expand Up @@ -111,6 +116,37 @@ func (a *AlertService) sendSlackTestNotification(ctx context.Context, url, notif
return nil
}

func (a *AlertService) sendSlackRequestAuthzNotification(ctx context.Context, url, notifySetting, defaultLocale, userName, projectName string, projectID uint32) error {
var setting slackNotifySetting
if err := json.Unmarshal([]byte(notifySetting), &setting); err != nil {
return err
}

var locale string
switch setting.Locale {
case LocaleJa:
locale = LocaleJa
case LocaleEn:
locale = LocaleEn
default:
locale = defaultLocale
}

if setting.WebhookURL != "" {
webhookMsg := getRequestAuthzWebhookMessage(setting.Data.Channel, locale, userName, projectName, url, projectID)
if err := slack.PostWebhook(setting.WebhookURL, webhookMsg); err != nil {
return fmt.Errorf("failed to send slack(webhookurl): %w", err)
}
} else if setting.ChannelID != "" {
if err := a.postMessageSlackWithRetry(ctx,
setting.ChannelID, slack.MsgOptionText(getRequestAuthzSlackMessageText(locale, userName, projectName, url, projectID), false)); err != nil {
return fmt.Errorf("failed to send slack(postmessage): %w", err)
}
}

return nil
}

func (a *AlertService) postMessageSlack(channelID string, msg ...slack.MsgOption) error {
if _, _, err := a.slackClient.PostMessage(channelID, msg...); err != nil {
var rateLimitError *slack.RateLimitedError
Expand Down Expand Up @@ -249,6 +285,28 @@ func getTestSlackMessageText(locale string) string {
return msgText
}

func getRequestAuthzWebhookMessage(channel, locale, projectName, userName, url string, projectID uint32) *slack.WebhookMessage {
msg := slack.WebhookMessage{
Text: getRequestAuthzSlackMessageText(locale, userName, projectName, url, projectID),
}
// override message
if channel != "" {
msg.Channel = channel
}
return &msg
}

func getRequestAuthzSlackMessageText(locale, projectName, userName, url string, projectID uint32) string {
var msgText string
switch locale {
case LocaleJa:
msgText = slackRequestAuthzNotificationMessageJa
default:
msgText = slackRequestAuthzNotificationMessageEn
}
return fmt.Sprintf(msgText, userName, projectName, url, projectID, userName)
}

func getColor(severity string) string {
switch severity {
case "high":
Expand Down
4 changes: 2 additions & 2 deletions proto/alert/entity.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading