Conversation
Add consentAPIMux
Add tests
Verify consent on desired endpoints Store consent on POST requests
S7evinK
force-pushed
the
s7evink/consent-tracking
branch
from
6d4153b to
74da1f0
Compare
…rite into s7evink/consent-tracking
userapi/inthttp/server.go
Outdated
| } | ||
| err := s.QueryPolicyVersion(req.Context(), &request, &response) | ||
| if err != nil { | ||
| return util.JSONResponse{Code: http.StatusBadRequest, JSON: &response} |
There was a problem hiding this comment.
Please be consistent with other code. This should be a 500, use util.ErrorResponse(err). Also, we seem to drop err to the floor?
setup/config/config_global.go
Outdated
| } | ||
|
|
||
| func (c *UserConsentOptions) Verify(configErrors *ConfigErrors, isMonolith bool) { | ||
| if c.Enabled { |
There was a problem hiding this comment.
Invert and outdent please:
if !c.Enabled {
return
}
// rest of code
setup/config/config_global.go
Outdated
| } | ||
|
|
||
| c.TextTemplates = textTemplate.Must(textTemplate.New("blockEventsError").Parse(c.BlockEventsError)) | ||
| c.TextTemplates = textTemplate.Must(c.TextTemplates.New("serverNoticeTemplate").Parse(c.ServerNoticeContent.Body)) |
There was a problem hiding this comment.
Won't this just replace the stuff we did on :307?
| localpart, _, err := gomatrixserverlib.SplitID('@', data.UserID) | ||
| if err != nil { | ||
| logrus.WithError(err).Error("unable to split username") | ||
| return &internalError |
There was a problem hiding this comment.
Probably should be a 4xx given the user can randomly set data.UserID.
| ReadOnly bool | ||
| } | ||
|
|
||
| func consent(writer http.ResponseWriter, req *http.Request, userAPI userapi.UserInternalAPI, cfg *config.ClientAPI) *util.JSONResponse { |
There was a problem hiding this comment.
Not a great function signature because it's not clear how precedence works between the return arg and writer. If I write to writer and then also return a util.JSONResponse, what happens?
| stmt = sqlutil.TxStmt(txn, stmt) | ||
| } | ||
| err = stmt.QueryRowContext(ctx, localPart).Scan(&policy) | ||
| return |
There was a problem hiding this comment.
Handle sql.ErrNoRows. We probably don't want to produce an error here.
| ) (userIDs []string, err error) { | ||
| stmt := s.batchSelectPrivacyPolicyStmt | ||
| if txn != nil { | ||
| stmt = sqlutil.TxStmt(txn, stmt) |
There was a problem hiding this comment.
Likewise, no need for the conditional.
| row := stmt.QueryRowContext(ctx, localpart) | ||
| err = row.Scan(&roomIDNull) | ||
| if err != nil { | ||
| return "", err |
There was a problem hiding this comment.
Handle sql.ErrNoRows. We probably don't want to produce an error here.
| if accountType != api.AccountTypeAppService { | ||
| _, err = sqlutil.TxStmt(txn, stmt).ExecContext(ctx, localpart, createdTimeMS, hash, nil, accountType) | ||
| } else { | ||
| _, err = sqlutil.TxStmt(txn, stmt).ExecContext(ctx, localpart, createdTimeMS, hash, appserviceID, accountType) |
There was a problem hiding this comment.
I don't understand why you removed this.
| stmt = sqlutil.TxStmt(txn, stmt) | ||
| } | ||
| err = stmt.QueryRowContext(ctx, localPart).Scan(&policy) | ||
| return |
|
We recently updated our contributing guidelines. This PR is being closed because it isn't a feature we want to maintain going forwards. |
3 participants
This adds the unspecified consent tracking.
Most parts are documented here