Events db #20
Events db #20
Conversation
Pull Request Test Coverage Report for Build 186
💛 - Coveralls |
cmd/keycloakb/keycloak_bridge.go
Outdated
| @@ -233,20 +234,25 @@ func main() { | |||
| //Ping() error | |||
There was a problem hiding this comment.
Should we remove this commented line ?
| @@ -233,20 +234,25 @@ func main() { | |||
| //Ping() error | |||
| Query(query string, args ...interface{}) (*sql.Rows, error) | |||
| QueryRow(query string, args ...interface{}) *sql.Row | |||
There was a problem hiding this comment.
Do we need Exec, Query and QueryRow in the interface here ? It seems it is not used there.
| @@ -118,6 +119,76 @@ func (c *adminComponent) AdminEvent(ctx context.Context, adminEvent *fb.AdminEve | |||
| } | |||
| } | |||
|
|
|||
| func addCTtypeToEvent(event map[string]string) map[string]string { | |||
There was a problem hiding this comment.
Maybe we could use a switch. I think it would be easier to read and avoid some not needed evaluations.
| //ACCOUNT_CREATED | ||
| if event["operationType"] == "CREATE" { | ||
| // check if the resourcePath starts with prefix users | ||
| if strings.HasPrefix(event["resourcePath"], "users") { |
There was a problem hiding this comment.
Is there multiple possible value ? That's why we use prefix instead of strict equality ?
There was a problem hiding this comment.
resourcePath is of the format "users/UUID..." or "roles/...."
pkg/event/component.go
Outdated
| @@ -150,15 +239,25 @@ func eventToMap(event *fb.Event) map[string]string { | |||
| eventMap["ipAddress"] = string(event.IpAddress()) | |||
| eventMap["error"] = string(event.Error()) | |||
|
|
|||
| var detailsString string | |||
| var detailsMap map[string]string | |||
There was a problem hiding this comment.
I think you have a warning there as you could do it in one line
pkg/event/module.go
Outdated
| //username - username of the user that is impacted by the action | ||
| username := "" // | ||
| ctEventType := m["ct_event_type"] // the ct event type is established before at the component level | ||
| kcEventType := m["type"] |
There was a problem hiding this comment.
A comment for each next ones would be good
pkg/event/module.go
Outdated
|
|
||
| _, err := cm.db.Exec(insertEvent, origin, realmName, agentUserID, agentUsername, userID, username, ctEventType, kcEventType, kcOperationType, clientID, additionalInfo) | ||
| // if ctEventType is not "", then record the events in MariaDB | ||
| if m["ct_event_type"] != "" { |
There was a problem hiding this comment.
would be easier to read, if you consider the case where == "" , then return nil.
pkg/event/module.go
Outdated
| //userId is in the resourcePath | ||
| if resourcePath, ok := m["resourcePath"]; ok { | ||
| reg := regexp.MustCompile(`[0-9a-fA-F]{8}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{4}\-[0-9a-fA-F]{12}`) | ||
| if strings.HasPrefix(resourcePath, "users") { |
There was a problem hiding this comment.
Is it possible to simplify it with? :
if m["resourcePath"] == "users"{
//....
}
There was a problem hiding this comment.
No, because resourcePath has as preffix "users" , followed by an UUID. It can also be "roles" followed also by an UUID
pkg/event/module.go
Outdated
| err := json.Unmarshal(eventAuthDetails, &h) | ||
|
|
||
| if err != nil { | ||
| fmt.Println(err) |
There was a problem hiding this comment.
fmt should not be used I think. we should use the logger instead.
There was a problem hiding this comment.
You are right; the logger is TBD
pkg/event/module.go
Outdated
| return err | ||
| } | ||
|
|
||
| // in authdetails part we can retrieve the client id, agent realm id, agent user id |
There was a problem hiding this comment.
What do you think about of the following alternative ? Would it be easier to read/maintain or not
| // in authdetails part we can retrieve the client id, agent realm id, agent user id | |
| for k, v := range h { | |
| switch k { | |
| case "clientId": | |
| clientId = h["clientId"] | |
| case "//////": | |
| /////// | |
| default: | |
| infoMap[k] = v | |
| } | |
| } |
Store audit events in the DB (Maria DB)