diff --git a/ORGANIZATION_SUPPORT.md b/ORGANIZATION_SUPPORT.md new file mode 100644 index 0000000..f9a3bd4 --- /dev/null +++ b/ORGANIZATION_SUPPORT.md @@ -0,0 +1,66 @@ +# VPM Organization Support Feature + +## Problem + +Currently, VPM only allows users to publish packages from repositories under their own GitHub account. The validation in `check_vcs()` function requires the repository URL to start with `https://github.com/{username}/`, where `username` is the logged-in user's GitHub username. + +This prevents users from publishing packages from GitHub organizations they belong to. + +## Solution Overview + +1. Add a new database table to store user's GitHub organization memberships +2. Fetch user's organizations during OAuth login +3. Modify `check_vcs()` to also accept organization URLs where the user is a member +4. Automatically use organization name as package prefix when publishing from org repos + +## Files Changed + +### New Files +- `src/entity/organization.v` - UserOrganization entity +- `src/repo/organization.v` - Database operations for organizations + +### Modified Files +- `src/auth.v` - Fetch user's organizations during OAuth login +- `src/usecase/package/packages.v` - Support organization URLs and prefixes +- `src/package.v` - Pass organization info to create function + +## How It Works + +1. When a user logs in via GitHub OAuth, we fetch their organization memberships using the GitHub API (`/user/orgs`) +2. Organizations are stored in the `UserOrganization` table +3. When creating a package: + - The URL is validated against both the user's account AND their organizations + - If the URL belongs to an organization, the package name uses the org name as prefix (e.g., `v-hono.hono` instead of `meiseayoung.hono`) + +## Example + +User `meiseayoung` is a member of the `v-hono` organization. + +Before this change: +- Publishing `https://github.com/v-hono/v-hono-core` would fail with "You must submit a package from your own account" + +After this change: +- Publishing `https://github.com/v-hono/v-hono-core` with name `hono` creates package `v-hono.hono` + +## Database Migration + +Run the following to create the new table: + +```sql +CREATE TABLE IF NOT EXISTS "UserOrganization" ( + id SERIAL PRIMARY KEY, + user_id INTEGER NOT NULL, + org_name VARCHAR(255) NOT NULL, + created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP +); + +CREATE INDEX idx_user_org_user_id ON "UserOrganization" (user_id); +``` + +## GitHub OAuth Scope + +Note: The GitHub OAuth app may need the `read:org` scope to access organization memberships. Update the OAuth authorization URL if needed: + +``` +https://github.com/login/oauth/authorize?response_type=code&client_id={CLIENT_ID}&scope=read:org +``` diff --git a/src/auth.v b/src/auth.v index d8a1148..c1a77eb 100644 --- a/src/auth.v +++ b/src/auth.v @@ -6,11 +6,16 @@ import json import vweb import entity { User } import lib.log +import repo struct GitHubUser { login string } +struct GitHubOrg { + login string +} + const random = 'qwertyuiopasdfghjklzxcvbnmQWERTYUIOPASDFGHJKLZXCVBNM1234567890' fn random_string(len int) string { @@ -66,6 +71,30 @@ fn (mut app App) oauth_cb() vweb.Result { random_id = app.db.q_string("select random_id from \"User\" where username='${login}' ") or { panic(err) } + + // Fetch user's GitHub organizations + orgs_resp := http.fetch( + url: 'https://api.github.com/user/orgs' + method: .get + header: http.new_header(key: .authorization, value: 'token ${token}') + ) or { + println('failed to fetch orgs: ${err}') + http.Response{} + } + + if orgs_resp.status_code == 200 { + gh_orgs := json.decode([]GitHubOrg, orgs_resp.body) or { [] } + mut org_names := []string{cap: gh_orgs.len} + for org in gh_orgs { + org_names << org.login + } + // Save organizations to database + orgs_repo := repo.organizations(app.db) + orgs_repo.save_user_organizations(user_id, org_names) or { + println('failed to save orgs: ${err}') + } + } + app.set_cookie( name: 'id' value: user_id.str() diff --git a/src/entity/organization.v b/src/entity/organization.v new file mode 100644 index 0000000..8806dbb --- /dev/null +++ b/src/entity/organization.v @@ -0,0 +1,12 @@ +module entity + +import time + +@[json: 'user_organization'] +pub struct UserOrganization { +pub mut: + id int @[primary; sql: serial] + user_id int + org_name string + created_at time.Time = time.now() +} diff --git a/src/package.v b/src/package.v index c493904..1e8ccd0 100644 --- a/src/package.v +++ b/src/package.v @@ -6,6 +6,7 @@ import lib.storage import lib.html import markdown import entity { Package } +import repo @['/new'] fn (mut app App) new() vweb.Result { @@ -15,7 +16,11 @@ fn (mut app App) new() vweb.Result { @['/create_package'; post] pub fn (mut app App) create_package(name string, url string, description string) vweb.Result { - app.packages().create(name, url, description, app.cur_user) or { + // Get user's organizations + orgs_repo := repo.organizations(app.db) + user_orgs := orgs_repo.get_user_org_names(app.cur_user.id) + + app.packages().create_with_orgs(name, url, description, app.cur_user, user_orgs) or { log.error() .add('error', err.str()) .add('url', url) diff --git a/src/repo/organization.v b/src/repo/organization.v new file mode 100644 index 0000000..d7ff571 --- /dev/null +++ b/src/repo/organization.v @@ -0,0 +1,61 @@ +module repo + +import orm +import entity { UserOrganization } + +pub struct OrganizationsRepo { +mut: + db orm.Connection @[required] +} + +pub fn migrate_organizations(db orm.Connection) ! { + sql db { + create table UserOrganization + }! +} + +pub fn organizations(db orm.Connection) OrganizationsRepo { + return OrganizationsRepo{ + db: db + } +} + +pub fn (o OrganizationsRepo) get_user_organizations(user_id int) []UserOrganization { + return sql o.db { + select from UserOrganization where user_id == user_id + } or { [] } +} + +pub fn (o OrganizationsRepo) get_user_org_names(user_id int) []string { + orgs := o.get_user_organizations(user_id) + mut names := []string{cap: orgs.len} + for org in orgs { + names << org.org_name + } + return names +} + +pub fn (o OrganizationsRepo) user_belongs_to_org(user_id int, org_name string) bool { + orgs := sql o.db { + select from UserOrganization where user_id == user_id && org_name == org_name + } or { [] } + return orgs.len > 0 +} + +pub fn (o OrganizationsRepo) save_user_organizations(user_id int, org_names []string) ! { + // Delete existing organizations for this user + sql o.db { + delete from UserOrganization where user_id == user_id + } or {} + + // Insert new organizations + for org_name in org_names { + org := UserOrganization{ + user_id: user_id + org_name: org_name + } + sql o.db { + insert org into UserOrganization + } or { continue } + } +} diff --git a/src/repo/organization_test.v b/src/repo/organization_test.v new file mode 100644 index 0000000..05f1c02 --- /dev/null +++ b/src/repo/organization_test.v @@ -0,0 +1,101 @@ +module repo + +import entity { UserOrganization } + +// Mock database for testing +struct MockOrmConnection { +mut: + orgs []UserOrganization +} + +// Test UserOrganization entity +fn test_user_organization_creation() { + org := UserOrganization{ + id: 1 + user_id: 100 + org_name: 'v-hono' + } + + assert org.id == 1 + assert org.user_id == 100 + assert org.org_name == 'v-hono' +} + +fn test_user_organization_multiple() { + orgs := [ + UserOrganization{ + id: 1 + user_id: 100 + org_name: 'v-hono' + }, + UserOrganization{ + id: 2 + user_id: 100 + org_name: 'vlang' + }, + UserOrganization{ + id: 3 + user_id: 100 + org_name: 'another-org' + }, + ] + + assert orgs.len == 3 + assert orgs[0].org_name == 'v-hono' + assert orgs[1].org_name == 'vlang' + assert orgs[2].org_name == 'another-org' +} + +// Test helper function to extract org names +fn test_extract_org_names() { + orgs := [ + UserOrganization{ + id: 1 + user_id: 100 + org_name: 'v-hono' + }, + UserOrganization{ + id: 2 + user_id: 100 + org_name: 'vlang' + }, + ] + + mut names := []string{cap: orgs.len} + for org in orgs { + names << org.org_name + } + + assert names.len == 2 + assert 'v-hono' in names + assert 'vlang' in names +} + +// Test user belongs to org logic +fn check_membership(orgs []UserOrganization, org_name string) bool { + for org in orgs { + if org.org_name == org_name { + return true + } + } + return false +} + +fn test_user_belongs_to_org_logic() { + orgs := [ + UserOrganization{ + id: 1 + user_id: 100 + org_name: 'v-hono' + }, + UserOrganization{ + id: 2 + user_id: 100 + org_name: 'vlang' + }, + ] + + assert check_membership(orgs, 'v-hono') == true + assert check_membership(orgs, 'vlang') == true + assert check_membership(orgs, 'other-org') == false +} diff --git a/src/repo/repo.v b/src/repo/repo.v index 5abad4a..28ebcc6 100644 --- a/src/repo/repo.v +++ b/src/repo/repo.v @@ -6,4 +6,5 @@ pub fn migrate(db orm.Connection) ! { migrate_categories(db)! migrate_packages(db)! migrate_users(db)! + migrate_organizations(db)! } diff --git a/src/usecase/package/packages.v b/src/usecase/package/packages.v index 33e17ee..5549f5c 100644 --- a/src/usecase/package/packages.v +++ b/src/usecase/package/packages.v @@ -55,7 +55,32 @@ pub interface UsersRepo { get_by_id(id int) ?User } +pub interface OrganizationsRepo { + get_user_org_names(user_id int) []string + user_belongs_to_org(user_id int, org_name string) bool +} + +// Extract owner from GitHub URL (e.g., "https://github.com/v-hono/repo" -> "v-hono") +fn extract_owner_from_url(url string) string { + // Remove protocol + mut path := url.replace('https://', '').replace('http://', '') + // Remove host + if path.starts_with('github.com/') { + path = path.replace('github.com/', '') + } + // Get first path segment (owner) + parts := path.split('/') + if parts.len > 0 { + return parts[0] + } + return '' +} + pub fn (u UseCase) create(name string, vcsUrl string, description string, user User) ! { + return u.create_with_orgs(name, vcsUrl, description, user, []) +} + +pub fn (u UseCase) create_with_orgs(name string, vcsUrl string, description string, user User, user_orgs []string) ! { name_lower := name.to_lower() log.info().add('name', name).msg('create package') if user.username == '' || !is_valid_mod_name(name_lower) { @@ -65,7 +90,7 @@ pub fn (u UseCase) create(name string, vcsUrl string, description string, user U url := vcsUrl.replace('<', '<').limit(max_package_url_len) log.info().add('url', name).msg('create package') - vcs_name := check_vcs(url, user.username) or { return err } + vcs_name := check_vcs_with_orgs(url, user.username, user_orgs) or { return err } resp := http.get(url) or { return error('Failed to fetch package URL') } if resp.status_code == 404 { @@ -82,8 +107,17 @@ pub fn (u UseCase) create(name string, vcsUrl string, description string, user U return error('This URL has already been submitted') } + // Determine package name prefix (user or organization) + owner := extract_owner_from_url(url) + mut pkg_prefix := user.username + + // If URL belongs to an organization the user is a member of, use org name as prefix + if owner != user.username && owner in user_orgs { + pkg_prefix = owner + } + u.packages.create_package(Package{ - name: user.username + '.' + name.limit(max_name_len) + name: pkg_prefix + '.' + name.limit(max_name_len) url: url description: description vcs: vcs_name.limit(3) @@ -187,6 +221,10 @@ pub fn (u UseCase) update_package_info(package_id int, name string, url string, } pub fn check_vcs(url string, username string) !string { + return check_vcs_with_orgs(url, username, []) +} + +pub fn check_vcs_with_orgs(url string, username string, user_orgs []string) !string { for vcs in allowed_vcs { for protocol in vcs.protocols { for host in vcs.hosts { @@ -194,12 +232,24 @@ pub fn check_vcs(url string, username string) !string { continue } - if !url.starts_with(vcs.format_url(protocol, host, username)) - && username != 'medvednikov' { - return error('You must submit a package from your own account') + // Check if URL belongs to user's account + if url.starts_with(vcs.format_url(protocol, host, username)) { + return vcs.name + } + + // Check if URL belongs to one of user's organizations + for org in user_orgs { + if url.starts_with(vcs.format_url(protocol, host, org)) { + return vcs.name + } + } + + // Special case for admin + if username == 'medvednikov' { + return vcs.name } - return vcs.name + return error('You must submit a package from your own account or an organization you belong to') } } } diff --git a/src/usecase/package/packages_test.v b/src/usecase/package/packages_test.v new file mode 100644 index 0000000..0622a0f --- /dev/null +++ b/src/usecase/package/packages_test.v @@ -0,0 +1,154 @@ +module package + +// Test helper function: extract_owner_from_url +fn test_extract_owner_from_url_https() { + assert extract_owner_from_url('https://github.com/v-hono/v-hono-core') == 'v-hono' + assert extract_owner_from_url('https://github.com/meiseayoung/my-package') == 'meiseayoung' + assert extract_owner_from_url('https://github.com/vlang/vpm') == 'vlang' +} + +fn test_extract_owner_from_url_http() { + assert extract_owner_from_url('http://github.com/v-hono/v-hono-core') == 'v-hono' + assert extract_owner_from_url('http://github.com/meiseayoung/my-package') == 'meiseayoung' +} + +fn test_extract_owner_from_url_edge_cases() { + // Empty URL + assert extract_owner_from_url('') == '' + // URL without path + assert extract_owner_from_url('https://github.com/') == '' + // URL with only owner + assert extract_owner_from_url('https://github.com/owner') == 'owner' +} + +// Test check_vcs function (backward compatibility) +fn test_check_vcs_own_account() { + // User can publish from their own account + result := check_vcs('https://github.com/meiseayoung/my-package', 'meiseayoung') or { + assert false, 'should not fail for own account' + return + } + assert result == 'github' +} + +fn test_check_vcs_other_account_fails() { + // User cannot publish from another user's account + check_vcs('https://github.com/other-user/package', 'meiseayoung') or { + assert err.msg().contains('own account') || err.msg().contains('organization') + return + } + assert false, 'should fail for other account' +} + +fn test_check_vcs_admin_bypass() { + // Admin (medvednikov) can publish from any account + result := check_vcs('https://github.com/any-user/package', 'medvednikov') or { + assert false, 'admin should be able to publish from any account' + return + } + assert result == 'github' +} + +fn test_check_vcs_unsupported_vcs() { + // Unsupported VCS should fail + check_vcs('https://gitlab.com/user/package', 'user') or { + assert err.msg().contains('unsupported') + return + } + assert false, 'should fail for unsupported vcs' +} + +// Test check_vcs_with_orgs function (new functionality) +fn test_check_vcs_with_orgs_own_account() { + // User can still publish from their own account + result := check_vcs_with_orgs('https://github.com/meiseayoung/my-package', 'meiseayoung', + []) or { + assert false, 'should not fail for own account' + return + } + assert result == 'github' +} + +fn test_check_vcs_with_orgs_member_org() { + // User can publish from organization they belong to + user_orgs := ['v-hono', 'another-org'] + result := check_vcs_with_orgs('https://github.com/v-hono/v-hono-core', 'meiseayoung', + user_orgs) or { + assert false, 'should not fail for member organization: ${err}' + return + } + assert result == 'github' +} + +fn test_check_vcs_with_orgs_non_member_org_fails() { + // User cannot publish from organization they don't belong to + user_orgs := ['my-org'] + check_vcs_with_orgs('https://github.com/other-org/package', 'meiseayoung', user_orgs) or { + assert err.msg().contains('own account') || err.msg().contains('organization') + return + } + assert false, 'should fail for non-member organization' +} + +fn test_check_vcs_with_orgs_empty_orgs() { + // With empty orgs list, should behave like check_vcs + check_vcs_with_orgs('https://github.com/some-org/package', 'meiseayoung', []) or { + assert err.msg().contains('own account') || err.msg().contains('organization') + return + } + assert false, 'should fail with empty orgs list' +} + +fn test_check_vcs_with_orgs_multiple_orgs() { + // User belongs to multiple organizations + user_orgs := ['org1', 'org2', 'v-hono', 'org3'] + + // Can publish from any of them + result1 := check_vcs_with_orgs('https://github.com/org1/package', 'user', user_orgs) or { + assert false, 'should work for org1' + return + } + assert result1 == 'github' + + result2 := check_vcs_with_orgs('https://github.com/v-hono/package', 'user', user_orgs) or { + assert false, 'should work for v-hono' + return + } + assert result2 == 'github' +} + +fn test_check_vcs_with_orgs_http_protocol() { + // Should work with http protocol too + user_orgs := ['v-hono'] + result := check_vcs_with_orgs('http://github.com/v-hono/package', 'meiseayoung', user_orgs) or { + assert false, 'should work with http protocol' + return + } + assert result == 'github' +} + +// Test is_valid_mod_name function +fn test_is_valid_mod_name_valid() { + assert is_valid_mod_name('hono') == true + assert is_valid_mod_name('my.package') == true + assert is_valid_mod_name('Package123') == true + assert is_valid_mod_name('ab') == true // minimum length +} + +fn test_is_valid_mod_name_invalid() { + assert is_valid_mod_name('a') == false // too short + assert is_valid_mod_name('') == false // empty + assert is_valid_mod_name('my-package') == false // contains hyphen + assert is_valid_mod_name('my_package') == false // contains underscore + assert is_valid_mod_name('my package') == false // contains space + assert is_valid_mod_name('@scope/package') == false // contains @ and / +} + +fn test_is_valid_mod_name_max_length() { + // max_name_len is 35 + valid_name := 'a'.repeat(35) + assert is_valid_mod_name(valid_name) == true + + invalid_name := 'a'.repeat(36) + assert is_valid_mod_name(invalid_name) == false +}