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

Update sample.http to support user creation then token generation from the new users #15

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

stevenknox
Copy link

Updated the sample.http to support the creation of a regular and admin user, authenticate using each of these users, then use their tokens for the requests. The original implementation had a hard coded token that wouldn't work on a fresh run.

The change required some small changes to the AuthApi file to handle setting IsAdmin during the user creation request.

The sample.http file can now have each request run in sequence to create users, login, add, view then finally delete (as admin) an item.

[Required]
public string Password { get; set; }

public bool IsAdmin { get; set; }
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not comfortable with this. I think the IsAdmin logic should stay server side. Even though this is a demo app 😄

Copy link
Author

Choose a reason for hiding this comment

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

Ah no worries, I could revert the CreateUser back to LoginInfo so users can only be created as a regular user via the API then seed the Admin user on startup? Or is that overkill for this project?

Copy link
Owner

Choose a reason for hiding this comment

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

Either that or the first user is admin with a comment describing that.

Copy link
Author

Choose a reason for hiding this comment

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

Reverted the insecure user creation back to the original implementation and seeded admin on startup. sample.http now works with the new approach

@stevenknox
Copy link
Author

Was the resolution to this ok or should I close? Didn't want to bump as you will have plenty on but seen you had made a few commits recently and maybe missed this or thought it was still pending on my side.

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.

2 participants