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
fix(query): pw complexity policy #2417
Conversation
bulk iterates as long as new events
…st statement if it has previous sequence 0
no savepoint if noop stmt, tests for stmt handler
* column in a single place * use projection for columns * query column with aliases * rename methods * remove unused code
policy_grpc "github.com/caos/zitadel/internal/api/grpc/policy" | ||
auth_pb "github.com/caos/zitadel/pkg/grpc/auth" | ||
) | ||
|
||
func (s *Server) GetMyPasswordComplexityPolicy(ctx context.Context, _ *auth_pb.GetMyPasswordComplexityPolicyRequest) (*auth_pb.GetMyPasswordComplexityPolicyResponse, error) { | ||
policy, err := s.repo.GetMyPasswordComplexityPolicy(ctx) | ||
policy, err := s.query.MyPasswordComplexityPolicy(ctx, authz.GetCtxData(ctx).UserID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should be named PasswordComplexityPoliicyByUserID because if you have the userid as param it is not my anymore.
I'm not sure if this does the right thing, because here you have the userid as param for this request, and on the mgmt api it is the orgid for the same request
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think userid doesn't make sense, i changed it to orgid
"github.com/caos/zitadel/internal/api/authz" | ||
"github.com/caos/zitadel/internal/api/grpc/object" | ||
policy_grpc "github.com/caos/zitadel/internal/api/grpc/policy" | ||
mgmt_pb "github.com/caos/zitadel/pkg/grpc/management" | ||
) | ||
|
||
func (s *Server) GetPasswordComplexityPolicy(ctx context.Context, req *mgmt_pb.GetPasswordComplexityPolicyRequest) (*mgmt_pb.GetPasswordComplexityPolicyResponse, error) { | ||
policy, err := s.org.GetPasswordComplexityPolicy(ctx) | ||
policy, err := s.query.MyPasswordComplexityPolicy(ctx, authz.GetCtxData(ctx).OrgID) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
GetPasswordComplexityPolicyByOrgID
Nor sure if this request does the right thing, here you have the orgID as param, before it was the userID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think org id is correct
internal/query/org.go
Outdated
func prepareOrgQuery() (sq.SelectBuilder, func(*sql.Row) (*Org, error)) { | ||
return sq.Select( | ||
OrgColumnID.identifier(), | ||
OrgColumnCreationDate.identifier(), | ||
OrgColumnChangeDate.identifier(), | ||
OrgColumnResourceOwner.identifier(), | ||
OrgColumnState.identifier(), | ||
OrgColumnSequence.identifier(), | ||
OrgColumnName.identifier(), | ||
OrgColumnDomain.identifier(), | ||
). | ||
From(orgsTable.identifier()).PlaceholderFormat(sq.Dollar), | ||
func(row *sql.Row) (*Org, error) { | ||
o := new(Org) | ||
err := row.Scan( | ||
&o.ID, | ||
&o.CreationDate, | ||
&o.ChangeDate, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private methods to the bottom
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
main merged
ComplexityPolicyHasUppercaseCol = "has_uppercase" | ||
ComplexityPolicyHasSymbolCol = "has_symbol" | ||
ComplexityPolicyHasNumberCol = "has_number" | ||
ComplexityPolicyIsDefaultCol = "is_default" //TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this todo mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted
ComplexityPolicyHasSymbolCol = "has_symbol" | ||
ComplexityPolicyHasNumberCol = "has_number" | ||
ComplexityPolicyIsDefaultCol = "is_default" //TODO | ||
ComplexityPolicyResourceOwnerCol = "resource_owner" //TODO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what does this todo mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
deleted
@@ -0,0 +1 @@ | |||
ALTER TABLE zitadel.projections.orgs RENAME COLUMN domain TO primary_domain; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change migration versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
migration removed
@@ -0,0 +1,15 @@ | |||
CREATE TABLE zitadel.projections.password_complexity_policies ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
change migrations versions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
version changed to be merged after #2411
// case *proj_pb.ProjectQuery_ProjectResourceOwnerQuery: | ||
// return query.NewProjectResourceOwnerSearchQuery(q.ProjectResourceOwnerQuery.ResourceOwner) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why did you comment this? I added this on request for max, because he needs this filter on the list granted projects
🎉 This PR is included in version 1.47.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
merge after #2342