Addressing comments in PR review

- added a bit of validation to admin create and update
- using protojson where possible in admin api
- fixing a few instances of admin -> acme in errors
This commit is contained in:
max furman 2021-07-06 17:14:13 -07:00
parent 5679c9933d
commit 1df21b9b6a
7 changed files with 43 additions and 16 deletions

View file

@ -18,6 +18,17 @@ type CreateAdminRequest struct {
// Validate validates a new-admin request body.
func (car *CreateAdminRequest) Validate() error {
if car.Subject == "" {
return admin.NewError(admin.ErrorBadRequestType, "subject cannot be empty")
}
if car.Provisioner == "" {
return admin.NewError(admin.ErrorBadRequestType, "provisioner cannot be empty")
}
switch car.Type {
case linkedca.Admin_SUPER_ADMIN, linkedca.Admin_ADMIN:
default:
return admin.NewError(admin.ErrorBadRequestType, "invalid value for admin type")
}
return nil
}
@ -34,6 +45,11 @@ type UpdateAdminRequest struct {
// Validate validates a new-admin request body.
func (uar *UpdateAdminRequest) Validate() error {
switch uar.Type {
case linkedca.Admin_SUPER_ADMIN, linkedca.Admin_ADMIN:
default:
return admin.NewError(admin.ErrorBadRequestType, "invalid value for admin type")
}
return nil
}
@ -52,7 +68,7 @@ func (h *Handler) GetAdmin(w http.ResponseWriter, r *http.Request) {
"admin %s not found", id))
return
}
api.JSON(w, adm)
api.ProtoJSON(w, adm)
}
// GetAdmins returns a segment of admins associated with the authority.
@ -104,7 +120,7 @@ func (h *Handler) CreateAdmin(w http.ResponseWriter, r *http.Request) {
return
}
api.JSON(w, adm)
api.ProtoJSONStatus(w, adm, http.StatusCreated)
}
// DeleteAdmin deletes admin.
@ -127,6 +143,11 @@ func (h *Handler) UpdateAdmin(w http.ResponseWriter, r *http.Request) {
return
}
if err := body.Validate(); err != nil {
api.WriteError(w, err)
return
}
id := chi.URLParam(r, "id")
adm, err := h.auth.UpdateAdmin(r.Context(), id, &linkedca.Admin{Type: body.Type})
@ -135,5 +156,5 @@ func (h *Handler) UpdateAdmin(w http.ResponseWriter, r *http.Request) {
return
}
api.JSON(w, adm)
api.ProtoJSON(w, adm)
}

View file

@ -14,7 +14,7 @@ type nextHTTP = func(http.ResponseWriter, *http.Request)
// is enabled before servicing requests.
func (h *Handler) requireAPIEnabled(next nextHTTP) nextHTTP {
return func(w http.ResponseWriter, r *http.Request) {
if h.db == nil {
if !h.auth.IsAdminAPIEnabled() {
api.WriteError(w, admin.NewError(admin.ErrorNotImplementedType,
"administration API not enabled"))
return

View file

@ -171,5 +171,5 @@ func (h *Handler) UpdateProvisioner(w http.ResponseWriter, r *http.Request) {
api.WriteError(w, err)
return
}
api.ProtoJSONStatus(w, nu, http.StatusOK)
api.ProtoJSON(w, nu)
}

View file

@ -15,7 +15,7 @@ var (
provisionersTable = []byte("provisioners")
)
// DB is a struct that implements the AcmeDB interface.
// DB is a struct that implements the AdminDB interface.
type DB struct {
db nosqlDB.DB
authorityID string
@ -54,7 +54,7 @@ func (db *DB) save(ctx context.Context, id string, nu interface{}, old interface
} else {
oldB, err = json.Marshal(old)
if err != nil {
return errors.Wrapf(err, "error marshaling acme type: %s, value: %v", typ, old)
return errors.Wrapf(err, "error marshaling admin type: %s, value: %v", typ, old)
}
}

View file

@ -12,7 +12,7 @@ import (
"github.com/smallstep/certificates/logging"
)
// ProblemType is the type of the ACME problem.
// ProblemType is the type of the Admin problem.
type ProblemType int
const (
@ -33,7 +33,7 @@ const (
ErrorServerInternalType
)
// String returns the string representation of the acme problem type,
// String returns the string representation of the admin problem type,
// fulfilling the Stringer interface.
func (ap ProblemType) String() string {
switch ap {
@ -73,17 +73,17 @@ var (
ErrorNotFoundType: {
typ: ErrorNotFoundType.String(),
details: "resource not found",
status: 400,
status: http.StatusNotFound,
},
ErrorAuthorityMismatchType: {
typ: ErrorAuthorityMismatchType.String(),
details: "resource not owned by authority",
status: 401,
status: http.StatusUnauthorized,
},
ErrorDeletedType: {
typ: ErrorDeletedType.String(),
details: "resource is deleted",
status: http.StatusUnauthorized,
status: http.StatusNotFound,
},
ErrorNotImplementedType: {
typ: ErrorNotImplementedType.String(),
@ -104,7 +104,7 @@ var (
}
)
// Error represents an ACME
// Error represents an Admin
type Error struct {
Type string `json:"type"`
Detail string `json:"detail"`

View file

@ -508,6 +508,12 @@ func (a *Authority) GetAdminDatabase() admin.DB {
return a.adminDB
}
// IsAdminAPIEnabled returns a boolean indicating whether the Admin API has
// been enabled.
func (a *Authority) IsAdminAPIEnabled() bool {
return a.config.AuthorityConfig.EnableAdmin
}
// Shutdown safely shuts down any clients, databases, etc. held by the Authority.
func (a *Authority) Shutdown() error {
if err := a.keyManager.Close(); err != nil {

View file

@ -132,7 +132,7 @@ retry:
return nil, readAdminError(resp.Body)
}
var adm = new(linkedca.Admin)
if err := readJSON(resp.Body, adm); err != nil {
if err := readProtoJSON(resp.Body, adm); err != nil {
return nil, errors.Wrapf(err, "error reading %s", u)
}
return adm, nil
@ -270,7 +270,7 @@ retry:
return nil, readAdminError(resp.Body)
}
var adm = new(linkedca.Admin)
if err := readJSON(resp.Body, adm); err != nil {
if err := readProtoJSON(resp.Body, adm); err != nil {
return nil, errors.Wrapf(err, "error reading %s", u)
}
return adm, nil
@ -334,7 +334,7 @@ retry:
return nil, readAdminError(resp.Body)
}
var adm = new(linkedca.Admin)
if err := readJSON(resp.Body, adm); err != nil {
if err := readProtoJSON(resp.Body, adm); err != nil {
return nil, errors.Wrapf(err, "error reading %s", u)
}
return adm, nil