2

I'm using a query with parameters to create a database user and SQL Server login. I have next procedure:

PROCEDURE [dbo].[regNewOfficial]
@name nvarchar(30),
@password nvarchar(30)
AS
DECLARE @sqlRequest NVARCHAR(1000);
SET @sqlRequest = 'CREATE LOGIN @nameP' +  
                    ' WITH PASSWORD = @passwordP' + 
                    ' , DEFAULT_DATABASE=[TreeBase]' +  
                    ' CREATE USER @nameP' + 
                    ' FOR LOGIN @nameP' +
                    ' ALTER ROLE official_role ADD MEMBER @nameP ;';
EXECUTE sp_executesql  @sqlRequest, N'@nameP nvarchar(30), @passwordP nvarchar(30)', @nameP = @name, @passwordP = @password;

And I get this error:

Incorrect syntax near "@nameP"

SOLUTION: Need to use QUOTENAME() when concatenating a string

Result of using parameters without QUOTENAME():

CREATE LOGIN @nameP WITH PASSWORD = @passwordP , DEFAULT_DATABASE=[TreeBase] CREATE USER @nameP FOR LOGIN @nameP ALTER ROLE official_role ADD MEMBER @nameP ;

With QUOTENAME():

CREATE LOGIN [aaaaa] WITH PASSWORD = 'bbbbb' , DEFAULT_DATABASE=[TreeBase]; CREATE USER [aaaaa] FOR LOGIN [aaaaa]; ALTER ROLE official_role ADD MEMBER [aaaaa];

ekiryuhin
  • 803
  • 4
  • 19
  • try to break this up into separate calls for the two CREATE and the one ALTER statement – user1443098 Apr 11 '18 at 13:17
  • 1
    I'm actually surprised that so many have voted to close. Especially as the topic being marked as a duplicate won't actually solve the OP's problem. I would kindly ask that others don't pile on that vote please. The OP has also showed what they tried, and given us an error; which can easily be reproduced. So it meets all the requirements of a good question. +1 :) – Larnu Apr 11 '18 at 13:30
  • Even if it is a duplicate, it's not a duplicate of [EXEC sp_executesql with multiple parameters](https://stackoverflow.com/questions/28481189/exec-sp-executesql-with-multiple-parameters) - Clearly the problem here is the same as in [Create users dynamic names and assgin roles](https://stackoverflow.com/questions/4991643/create-users-dynamic-names-and-assgin-roles). However, the accepted answer on that post suggests using a stored procedure that since was deprecated, so I will not recommend this as a duplicate also. – Zohar Peled Apr 11 '18 at 14:34
  • Ha! It's magic! Closed as duplicate and reopened, and all the close as duplicate votes are gone! – Zohar Peled Apr 11 '18 at 14:38
  • Please don't post the solution as a part of the question. You've already accepted the solution posted by @Larnu. If you decided to do something else, you can always post an answer to your own question, specifying the solution you've chosen. – Zohar Peled Apr 11 '18 at 14:41
  • Also, the reason you've said it works now is because of `QUOTENAME`, which isn't true at all. `QUOTENAME` is there to stop SQL injection. Youu *could* remove it and just concatenate your string, however, you would be a fool to do so. The reason it works is because it puts string literals into the dynamic SQL; the generated SQL doesn't have variables. – Larnu Apr 11 '18 at 18:37

1 Answers1

3

CREATE LOGIN requires literal strings to work; you can't pass it variables. Therefore you need to do your Dynamic SQL in a different way.

ALTER PROCEDURE [dbo].[regNewOfficial]
    @name nvarchar(30),
    @password nvarchar(30)
AS

    DECLARE @sqlRequest NVARCHAR(1000);
    SET @sqlRequest = N'CREATE LOGIN ' + QUOTENAME(@name) +
                      N' WITH PASSWORD = ' + QUOTENAME(@password,'''') +
                      N' , DEFAULT_DATABASE=[TreeBase]' + N';' + NCHAR(10) +  
                      N' CREATE USER ' + QUOTENAME(@name) +
                      N' FOR LOGIN ' + QUOTENAME(@name) + N';' + NCHAR(10) + 
                      N' ALTER ROLE official_role ADD MEMBER ' + QUOTENAME(@name) + N';';
    PRINT @sqlRequest; --This is your friend for debugging
    EXECUTE sp_executesql @sqlRequest;
GO
Larnu
  • 61,056
  • 10
  • 27
  • 50
  • 1
    `PRINT @sqlRequest; --This is your friend for debugging` This alone is worth an upvote for being a good advice and yet funny at the same time. Also, I think your answer is correct. – Zohar Peled Apr 11 '18 at 13:22
  • @ZoharPeled I wouldn't be surprised if `PRINT @SQL;` has saved me more hours of development time over the years than anything else. – Larnu Apr 11 '18 at 13:27
  • I totally agree - Whenever you write dynamic SQL - `print @sql;` is your best friend. I just didn't expect that comment (usually I write things like "delete this row once you get the sql right"), so it made me laugh. – Zohar Peled Apr 11 '18 at 14:30