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

Add database schema #7

Merged
merged 11 commits into from
Nov 16, 2023
Merged

Add database schema #7

merged 11 commits into from
Nov 16, 2023

Conversation

WizzyGeek
Copy link
Contributor

Adds normalised database models.

@WizzyGeek WizzyGeek marked this pull request as draft November 14, 2023 12:15
@parrothacker1
Copy link
Contributor

Bhaiyya do we need a team id field in the team model ? Cuz we are using name as the primary key in the team model.Should I make a new commit for that ?

@WizzyGeek
Copy link
Contributor Author

@Meetesh-Saini
@mradigen
@parrothacker1

I am trying to achieve a 2NF database schema
Can you check if these models cover all our database requirements? (except the User and Team models, I do not know the requirements for that @parrothacker1 do let me know)

@Meetesh-Saini Meetesh-Saini marked this pull request as ready for review November 15, 2023 09:19
@Meetesh-Saini Meetesh-Saini marked this pull request as draft November 15, 2023 09:20
@Meetesh-Saini
Copy link
Member

Problem doesn't have any primary key. Add problem id for it. Also we can merge Image to Problem

@mradigen
Copy link
Member

Problem doesn't have any primary key. Add problem id for it. Also we can merge Image to Problem

Tortoise adds an ID of its own if no primary key is mentioned

@WizzyGeek
Copy link
Contributor Author

All of the Models without a primary key rely on the default from the ORM

And I liked to remain high level and let the orm manage the exact details of relations

@WizzyGeek
Copy link
Contributor Author

WizzyGeek commented Nov 15, 2023

About the team having a max of N users

I tried two approaches

DO $$ 
BEGIN 
    IF NOT EXISTS (
        SELECT 1 
        FROM pg_constraint 
        WHERE conname = 'max_users_per_team_constraint' 
    ) THEN
        -- Add the constraint only if it doesn't exist
        ALTER TABLE "user" 
        ADD CONSTRAINT max_users_per_team_constraint 
        CHECK ((SELECT COUNT(*) FROM "user" WHERE team_id = "user".team_id) <= 3);
    END IF;
END $$;

but that lead to

asyncpg.exceptions.FeatureNotSupportedError: cannot use subquery in check constraint

So I setup a trigger on the DB

CREATE OR REPLACE FUNCTION check_max_users_per_team()
RETURNS TRIGGER AS $$
BEGIN
    IF (
        SELECT COUNT(*) 
        FROM "user" 
        WHERE team_id = NEW.team_id
    ) >= 3 THEN
        RAISE EXCEPTION 'Cannot have more than 3 users in a team';
    END IF;
    RETURN NEW;
END;
$$ LANGUAGE plpgsql;

DROP TRIGGER IF EXISTS max_users_per_team_trigger ON "user";

CREATE TRIGGER max_users_per_team_trigger
BEFORE INSERT OR UPDATE ON "user"
FOR EACH ROW EXECUTE FUNCTION check_max_users_per_team();

image

@WizzyGeek
Copy link
Contributor Author

I wrote 3 over there but I will just inject the actual value

@WizzyGeek
Copy link
Contributor Author

WizzyGeek commented Nov 15, 2023

Now trying to create more than 3 Users for one team leads to an Error from the driver

await User.create(user_name="345", team_id=2)

Edit (after new commits):

await User.create(name="345", team_id=2)
asyncpg.exceptions.RaiseError: Cannot have more than 3 users in a team

@WizzyGeek
Copy link
Contributor Author

Now the question is,

  • Use trigger or
  • Use application side validation

fix: change column names to not contain model names
@WizzyGeek WizzyGeek marked this pull request as ready for review November 15, 2023 13:06
@WizzyGeek
Copy link
Contributor Author

All has been tested, now only missing annotation for image_config and constraint checking based on config value

src/pwncore/models/ctf.py Show resolved Hide resolved
@mradigen
Copy link
Member

All has been tested, now only missing annotation for image_config and constraint checking based on config value

Currently the image_config is the raw POST data that is sent to the Docker Remote API. For now it only contains the ports required to be opened on the host, but I've left it as JSON for flexibility in case a CTF requires custom network configs.

@Meetesh-Saini Meetesh-Saini merged commit ba01437 into lugvitc:master Nov 16, 2023
2 checks passed
mradigen added a commit to mradigen/pwncore that referenced this pull request Nov 16, 2023
mradigen added a commit to mradigen/pwncore that referenced this pull request Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants