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

Tapir based secuirty #794

Merged
merged 7 commits into from
Feb 3, 2022
Merged

Tapir based secuirty #794

merged 7 commits into from
Feb 3, 2022

Conversation

Pask423
Copy link
Contributor

@Pask423 Pask423 commented Jan 30, 2022

No description provided.

@Pask423
Copy link
Contributor Author

Pask423 commented Jan 30, 2022

closes #769

@Pask423
Copy link
Contributor Author

Pask423 commented Jan 30, 2022

closes #793

@Pask423 Pask423 requested a review from adamw January 30, 2022 14:33
.in(jsonBody[ChangePassword_IN])
.out(jsonBody[ChangePassword_OUT])
.serverLogic { case (authData, data) =>
.serverSecurityLogic(authData => auth.applyEither(authData).map(_.left.map(exceptionToErrorOut)))
Copy link
Member

Choose a reason for hiding this comment

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

can't we use .toOut here as for server logic?

Copy link
Contributor Author

@Pask423 Pask423 Jan 31, 2022

Choose a reason for hiding this comment

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

Not if I understand it correctly here we just mapping left while .toOut perform mapping on both left and right, in fact .toOut maps whole IO[] we just need to map the left of Either

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't understand correctly fixed

(for {
_ <- userService.changePassword(id, data.currentPassword, data.newPassword).transact(xa)
} yield ChangePassword_OUT()).toOut
)

private val getUserEndpoint = secureEndpoint.get
.in(UserPath)
Copy link
Member

Choose a reason for hiding this comment

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

if changePasswordEndpoint uses .securityIn for the path, shouldn't we use this here as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Now I am not sure if changePasswordEndpoint require path in .securityIn , which way is better in your opinion ?

Copy link
Member

Choose a reason for hiding this comment

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

yeah you're right it's fine to keep them all in the regular inputs. The path matching is done on both anyway

@Pask423
Copy link
Contributor Author

Pask423 commented Feb 2, 2022

closes #796
Probably also can close #270 because newest version of otj-pg-embedded includes tests containers.
@adamw could you verify if such setting is enough to close #270 ?

@adamw adamw merged commit 50ec550 into master Feb 3, 2022
@adamw
Copy link
Member

adamw commented Feb 3, 2022

Thanks :)

@adamw
Copy link
Member

adamw commented Feb 3, 2022

@Pask423 so right now postgres is run through a container?

@mergify mergify bot deleted the tapir-based-secuirty branch February 3, 2022 12:39
@adamw
Copy link
Member

adamw commented Feb 3, 2022

@Pask423 indeed, it seems #270 is done also :)

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