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

fix(query): pw complexity policy #2417

Merged
merged 194 commits into from Oct 20, 2021
Merged

fix(query): pw complexity policy #2417

merged 194 commits into from Oct 20, 2021

Conversation

adlerhurst
Copy link
Member

@adlerhurst adlerhurst commented Sep 23, 2021

merge after #2342

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)
Copy link
Member

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

Copy link
Member Author

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)
Copy link
Member

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

Copy link
Member Author

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

Comment on lines 53 to 70
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,
Copy link
Member

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

Copy link
Member Author

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
Copy link
Member

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?

Copy link
Member Author

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
Copy link
Member

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?

Copy link
Member Author

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;
Copy link
Member

Choose a reason for hiding this comment

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

Change migration versions

Copy link
Member Author

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 (
Copy link
Member

Choose a reason for hiding this comment

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

change migrations versions

Copy link
Member Author

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

@adlerhurst adlerhurst marked this pull request as ready for review October 19, 2021 13:09
Comment on lines 71 to 72
// case *proj_pb.ProjectQuery_ProjectResourceOwnerQuery:
// return query.NewProjectResourceOwnerSearchQuery(q.ProjectResourceOwnerQuery.ResourceOwner)
Copy link
Member

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

@hifabienne hifabienne merged commit d7c681e into main Oct 20, 2021
@hifabienne hifabienne deleted the pw-complexity-policy branch October 20, 2021 11:44
@github-actions
Copy link

🎉 This PR is included in version 1.47.3 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants