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

Move the DROP TRIGGER so that it is executed prior to CREATE of a new trigger, to avoid granting excessive permissions to end users #170

Closed
CashmoreP opened this issue Dec 16, 2019 · 9 comments
Assignees
Labels
Milestone

Comments

@CashmoreP
Copy link

Rather than having a DROP TRIGGER inside the CREATE TRIGGER code, could the drop be performed prior to the CREATE TRIGGER, e.g.

get a list of all triggers from sys.triggers matching tr_{schema}{tableName}*_Sender
drop each trigger matching this
then create the new trigger

The current approach means that security has to be changed to grant DROP TRIGGER permission to any user who may update the table. The alternative approach means that DROP TRIGGER permission can be restricted to the user who would already have CREATE TRIGGER permissions.

@christiandelbianco
Copy link
Collaborator

Hi CashmoreP ,
can you explain me better your idea. I do not understand...Thanks

@CashmoreP
Copy link
Author

Consider the following scenario:

User 1 has permission to create and drop triggers
User 2 has no permission to create and drop triggers

If user 2 attempts to update data in a table that has a tr_{schema}{tableName}*_Sender trigger on it, the trigger SQL code could result in an attempt to invoke a DROP TRIGGER command, which fails because user 2 has not got sufficient permissions.

The solution is to remove the DROP TRIGGER call from the tr_{schema}{tableName}*_Sender trigger and drop the trigger in a different area of code. I can get one of my colleagues to commit changes so that you can see the end result.

@christiandelbianco christiandelbianco self-assigned this Jan 7, 2020
@christiandelbianco christiandelbianco added this to the 8.5.7 milestone Jan 7, 2020
@christiandelbianco
Copy link
Collaborator

@CashmoreP Yes, you are perfectly right. Good suggestion!!!
As you probably already have fixed it, can you tell me the point where you think is better to move the code that drop the trigger?

    DECLARE @conversationHandlerExists INT
    SELECT @conversationHandlerExists = COUNT(*) FROM sys.conversation_endpoints WHERE conversation_handle = '3259dc30-8931-ea11-a5bc-9cb6d0c4de6c';
    IF @conversationHandlerExists = 0
    BEGIN
        DROP TRIGGER [tr_dbo_Products_950f74d5-2f11-493a-a325-d992d1040cb3_Sender];
        RETURN
    END

@CashmoreP
Copy link
Author

CashmoreP commented Jan 7, 2020 via email

@christiandelbianco
Copy link
Collaborator

Thanks. As usual, to anyone that find a bug and also propose a suggestion, I ask if can i put his/their name(s) in contributors page. So, if for you is ok, just write me some words (if you want) about your profile as done by the other developers.

@CashmoreP
Copy link
Author

CashmoreP commented Jan 7, 2020 via email

@CashmoreP
Copy link
Author

CashmoreP commented Jan 8, 2020 via email

@christiandelbianco
Copy link
Collaborator

Added clause EXECUTE AS SELF to trigger, in order to run it in the context of the creator.

@christiandelbianco
Copy link
Collaborator

Fix available in release 8.5.7

christiandelbianco added a commit that referenced this issue Jan 11, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
SqlTableDependency
  
Awaiting triage
Development

No branches or pull requests

2 participants