-
Notifications
You must be signed in to change notification settings - Fork 385
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
satellite/console: payments api #3297
Conversation
|
||
err = json.NewEncoder(w).Encode(balanceResponse) | ||
if err != nil { | ||
p.serveJSONError(w, http.StatusInternalServerError, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the json NewEncoder already started writing something, it will double write the header.
mux.Handle("/api/v0/token", http.HandlerFunc(server.tokenHandler)) | ||
paymentController := consoleapi.NewPayments(logger, service) | ||
paymentsRouter := router.PathPrefix("/api/v0/payments").Subrouter() | ||
paymentsRouter.Handle("/cards", http.HandlerFunc(paymentController.AddCreditCard)).Methods(http.MethodPost) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, just noticed this... mux.Router has HandleFunc
, so you don't have to cast everything to HandlerFunc
satellite/api.go
Outdated
@@ -363,11 +364,16 @@ func NewAPI(log *zap.Logger, full *identity.FullIdentity, db DB, pointerDB metai | |||
if consoleConfig.AuthTokenSecret == "" { | |||
return nil, errs.New("Auth token secret required") | |||
} | |||
|
|||
paymentsConfig := stripecoinpayments.Config{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we have to create config here?
var err error | ||
defer mon.Task()(&ctx)(&err) | ||
|
||
balance, err := p.service.Payments().AccountBalance(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing authorization
return | ||
} | ||
|
||
err = p.service.Payments().AddCreditCard(ctx, requestBody.Token) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing authorization
satellite/peer.go
Outdated
@@ -592,11 +592,15 @@ func New(log *zap.Logger, full *identity.FullIdentity, db DB, pointerDB metainfo | |||
return nil, errs.New("Auth token secret required") | |||
} | |||
|
|||
paymentsConfig := stripecoinpayments.Config{} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we create config here
var err error | ||
defer mon.Task()(&ctx)(&err) | ||
|
||
err = p.service.Payments().SetupAccount(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing authorization
|
||
// authorize checks request for authorization token, validates it and updates context with auth data. | ||
func (p *Payments) authorize(ctx context.Context, r *http.Request) (context.Context, error) { | ||
tokenCookie, err := r.Cookie("_tokenKey") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
api doesn't use coockie auth
} | ||
|
||
auth, err := p.service.Authorize(auth.WithAPIKey(ctx, []byte(tokenCookie.Value))) | ||
if err != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if err add error to context
satellite/api.go
Outdated
@@ -363,11 +364,16 @@ func NewAPI(log *zap.Logger, full *identity.FullIdentity, db DB, pointerDB metai | |||
if consoleConfig.AuthTokenSecret == "" { | |||
return nil, errs.New("Auth token secret required") | |||
} | |||
|
|||
paymentsConfig := config.Payments | |||
payments := stripecoinpayments.NewService(paymentsConfig, peer.DB.Customers()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use config.Payments
directly
@@ -49,11 +50,15 @@ func TestGrapqhlMutation(t *testing.T) { | |||
|
|||
log := zaptest.NewLogger(t) | |||
|
|||
paymentsConfig := stripecoinpayments.Config{} | |||
payments := stripecoinpayments.NewService(paymentsConfig, db.Customers()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use config.Payments
directly
|
||
passwordCost int | ||
} | ||
|
||
// PaymentsService separates all payment related functionality | ||
type PaymentsService struct { | ||
service *Service |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
smth with console
would make more sense
What:
satellite/console/service
extended with payments functionalityWhy:
Please describe the tests:
Please describe the performance impact:
Code Review Checklist (to be filled out by reviewer)