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

[WIP] : Multiple project owner backend. #4536

Conversation

aryan-bhokare
Copy link
Contributor

@aryan-bhokare aryan-bhokare commented Mar 17, 2024

closes #3958 #4161

Proposed changes

Work done:

  1. Modification of db schema.
  2. Addition of new API GetProjectOwners.
  3. Modifications of SendInvitation API.
  4. Modifications of LeaveProject API.
  5. Added new API UpdateMemberRole.
  6. Added new API DeleteProject.

Types of changes

What types of changes does your code introduce to Litmus? Put an x in the boxes that apply

  • New feature (non-breaking change which adds functionality)
  • Bugfix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Update (if none of the other choices applies)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the CONTRIBUTING doc
  • I have signed the commit for DCO to be passed.
  • Lint and unit tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works (if appropriate)
  • I have added necessary documentation (if appropriate)

Dependency

  • Please add the links to the dependent PR need to be merged before this (if any).

Special notes for your reviewer:

Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
var project struct {
Members []*entities.Member `bson:"members"`
}
err := r.Collection.FindOne(context.TODO(), filter).Decode(&project)

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources

This query depends on a [user-provided value](1).
dependabot bot and others added 5 commits March 23, 2024 11:42
…itmuschaos#4527)

Bumps [golang.org/x/crypto](https://github.com/golang/crypto) from 0.18.0 to 0.21.0.
- [Commits](golang/crypto@v0.18.0...v0.21.0)

---
updated-dependencies:
- dependency-name: golang.org/x/crypto
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
)

Bumps [follow-redirects](https://github.com/follow-redirects/follow-redirects) from 1.15.5 to 1.15.6.
- [Release notes](https://github.com/follow-redirects/follow-redirects/releases)
- [Commits](follow-redirects/follow-redirects@v1.15.5...v1.15.6)

---
updated-dependencies:
- dependency-name: follow-redirects
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [github.com/golang/protobuf](https://github.com/golang/protobuf) from 1.5.3 to 1.5.4.
- [Release notes](https://github.com/golang/protobuf/releases)
- [Commits](golang/protobuf@v1.5.3...v1.5.4)

---
updated-dependencies:
- dependency-name: github.com/golang/protobuf
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Raj Das <mail.rajdas@gmail.com>
This modification unables to send invite with the role as owner.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
This modification checks if the User is the last owner of the project and if not User can leave the project.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
query := bson.D{{"_id", projectID}}
update := bson.D{{"$set", bson.M{"members.$[elem]role": role}}}

_, err := r.Collection.UpdateOne(context.TODO(), query, update, opts)

Check failure

Code scanning / CodeQL

Database query built from user-controlled sources

This query depends on a [user-provided value](1).
Allows Owner to be able to leave the project.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
This API is used for updating role of the member in the project.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
func GetActiveProjectOwners(service services.ApplicationService) gin.HandlerFunc {
return func(c *gin.Context) {
projectID := c.Param("project_id")
// state := c.Param("state")
Copy link
Contributor

Choose a reason for hiding this comment

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

can remove if not required

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure

version "1.15.5"
resolved "https://registry.yarnpkg.com/follow-redirects/-/follow-redirects-1.15.5.tgz#54d4d6d062c0fa7d9d17feb008461550e3ba8020"
integrity sha512-vSFWUON1B+yAw1VN4xMfxgn5fTUiaOzAJCKBwIIgT/+7CuGy9+r+5gITvP62j3RmaD5Ph65UaERdOSRGUzZtgw==
version "1.15.6"
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this change required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It happened automatically. I am unable to understand I was thinking about starting new from the commit before the bumps

Copy link
Contributor Author

Choose a reason for hiding this comment

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

these changes were included in the main litmus repo I will remove it from the pr.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
@@ -569,6 +593,20 @@ func LeaveProject(service services.ApplicationService) gin.HandlerFunc {
return
}

if member.Role == nil || *member.Role == entities.RoleOwner {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we checking member.Role == nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because if we don't, it gives a runtime error attempting to access or deference nil pointer

Copy link
Contributor Author

Choose a reason for hiding this comment

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

2024/04/09 14:28:53 [Recovery] 2024/04/09 - 14:28:53 panic recovered:
runtime error: invalid memory address or nil pointer dereference
/usr/local/go/src/runtime/panic.go:261 (0x4556d7)
	panicmem: panic(memoryError)
/usr/local/go/src/runtime/signal_unix.go:881 (0x4556a5)
	sigpanic: panicmem()
/home/aryan/Desktop/dev2/lfx/litmus/chaoscenter/authentication/api/handlers/rest/project_handler.go:595 (0xc540df)
	ProjectRouter.LeaveProject.func16: if *member.Role == entities.RoleOwner {
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174 (0xc1972a)
	(*Context).Next: c.handlers[c.index](c)
/home/aryan/Desktop/dev2/lfx/litmus/chaoscenter/authentication/api/middleware/jwt_middlware.go:33 (0xc5824e)
	ProjectRouter.JwtMiddleware.func1: c.Next()
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174 (0xc1972a)
	(*Context).Next: c.handlers[c.index](c)
/home/aryan/Desktop/dev2/lfx/litmus/chaoscenter/authentication/api/middleware/jwt_middlware.go:33 (0xc5b54e)
	UserRouter.JwtMiddleware.func3: c.Next()
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174 (0xc26619)
	(*Context).Next: c.handlers[c.index](c)
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/recovery.go:102 (0xc26607)
	CustomRecoveryWithWriter.func1: c.Next()
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174 (0xc2575c)
	(*Context).Next: c.handlers[c.index](c)
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/logger.go:240 (0xc25743)
	LoggerWithConfig.func1: c.Next()
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/context.go:174 (0xc24c4d)
	(*Context).Next: c.handlers[c.index](c)
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:620 (0xc248dc)
	(*Engine).handleHTTPRequest: c.Next()
/home/aryan/go/pkg/mod/github.com/gin-gonic/gin@v1.9.1/gin.go:576 (0xc24411)
	(*Engine).ServeHTTP: engine.handleHTTPRequest(c)
/usr/local/go/src/net/http/server.go:3137 (0x800ced)
	serverHandler.ServeHTTP: handler.ServeHTTP(rw, req)
/usr/local/go/src/net/http/server.go:2039 (0x7fbfc7)
	(*conn).serve: serverHandler{c.server}.ServeHTTP(w, w.req)
/usr/local/go/src/runtime/asm_amd64.s:1695 (0x472c20)
	goexit: BYTE	$0x90	// NOP


}

err = service.UpdateMemberRole(member.ProjectID, member.UserID, member.Role)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should also add one more check for a case: There is only 1 owner and that owner is trying update their role to viewer/editor. In this case there will be no project owner. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure I'll do that

Copy link
Contributor

Choose a reason for hiding this comment

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

How about a check to not allow the logged in user to change their own project roles. This will cover all the conditions. Please check on feasibility of this approach and race conditions if any

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes that would be better. cc: @aryan-bhokare

@Saranya-jena Saranya-jena changed the base branch from master to multiple-owner-support April 9, 2024 06:25
@Saranya-jena Saranya-jena added the LFX-MENTORSHIP Linux Foundation Mentor ship Issue label Apr 9, 2024
aryan-bhokare and others added 3 commits April 9, 2024 13:27
Owner can delete project with help of this API.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
…kend

Signed-off-by: Aryan Bhokare <92683836+aryan-bhokare@users.noreply.github.com>
@aryan-bhokare aryan-bhokare marked this pull request as ready for review April 9, 2024 08:26
Signed-off-by: aryan <aryan1bhokare@gmail.com>
// @Router /delete_project [post]
//
// DeleteProject is used to delete a project.
func DeleteProject (service services.ApplicationService) gin.HandlerFunc {
Copy link
Contributor

Choose a reason for hiding this comment

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

All the resources related to the deleted project should also be deleted. This change can go in a different PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Lets do that in different pr.

User cannot change role of their own, so that it will avoid edge cases like
1. User is the last owner of the project.
2. User accidentally losing owner access to the projects.

Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
Signed-off-by: aryan <aryan1bhokare@gmail.com>
Copy link
Contributor

@Saranya-jena Saranya-jena left a comment

Choose a reason for hiding this comment

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

looks good to me 🚀

Copy link
Contributor

@SarthakJain26 SarthakJain26 left a comment

Choose a reason for hiding this comment

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

Looks good to me 🚀 , great work

@aryan-bhokare
Copy link
Contributor Author

Looks good to me 🚀 , great work

Thank you 🚀

@Saranya-jena Saranya-jena merged commit 0e51dbf into litmuschaos:multiple-owner-support Apr 12, 2024
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LFX-MENTORSHIP Linux Foundation Mentor ship Issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support multiple project owners
3 participants