Skip to content

Remove security.User from auditing and split http/grpc/connect#206

Merged
majst01 merged 5 commits into
masterfrom
remove-security-from-auditing
Apr 22, 2026
Merged

Remove security.User from auditing and split http/grpc/connect#206
majst01 merged 5 commits into
masterfrom
remove-security-from-auditing

Conversation

@majst01
Copy link
Copy Markdown
Contributor

@majst01 majst01 commented Apr 8, 2026

Description

With MEP-4 we want to get rid of github.com/emicklei/go-restful but with auditing we inherit this dependency.

Therefore we need to:

  • split the http and connect/grpc auditing interceptor into separate packages
  • split api and impl
  • move security.User to a seperate struct into auditing/api as well as the helpers to get/store the user from/to context

See

Used AI-Tools ✨

  • none used for generation

@majst01 majst01 requested a review from a team as a code owner April 8, 2026 14:40
@majst01 majst01 self-assigned this Apr 8, 2026
@metal-robot metal-robot Bot added this to Development Apr 8, 2026
@majst01 majst01 requested review from Gerrit91 and vknabel and removed request for a team April 8, 2026 14:41
@majst01 majst01 marked this pull request as draft April 8, 2026 14:46
@majst01 majst01 force-pushed the remove-security-from-auditing branch from f56382a to 4dd9033 Compare April 9, 2026 05:39
@majst01 majst01 marked this pull request as ready for review April 9, 2026 05:41
Comment thread auditing/api.go Outdated
type User struct {
EMail string
Name string
Groups []string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the audit backend I don't think we need this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread auditing/api.go Outdated
Groups []string
Tenant string
Project string
Issuer string
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This can also be dropped.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread auditing/grpc/auditing-interceptor.go Outdated
Include string = "include-to-auditing"
// Exclude explicitly excludes the request to the auditing backend even if the request method would audit the request (only applies for the http filter)
Exclude string = "exclude-from-auditing"
RequestLoggerKey Key = iota
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As this seems to come from the rest package with a logger that can only be used with go-restful, I think we can remove it from this package. The RequestIDKey we can keep as it allows people to access the generated ID or provide an own request ID.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

Comment thread auditing/grpc/auditing-interceptor.go Outdated
err = a.Index(auditReqContext)
return resp, err
}, nil
type auditingConnectInterceptor struct {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Would it make sense to also split apart the connect interceptors to make files a bit smaller and to allow pure gRPC people to use them without having to import connect as a dependency to their projects?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

was thinking about that as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

@majst01 majst01 merged commit 482e6ed into master Apr 22, 2026
2 checks passed
@majst01 majst01 deleted the remove-security-from-auditing branch April 22, 2026 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants