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.

Sign up for GitHub

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

[csr] add private_server certificate type #2

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

[csr] add private_server certificate type #2

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c0944e9
[csr] add `private_server` certificate type
capcom6 Mar 7, 2025
be66001
[csr] add some tests
capcom6 Mar 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ module github.com/android-sms-gateway/ca
go 1.23.2

require (
github.com/android-sms-gateway/client-go v1.5.0
github.com/android-sms-gateway/client-go v1.5.5-0.20250307091924-1076d115eb88
github.com/go-playground/validator/v10 v10.23.0
github.com/gofiber/contrib/fiberzap/v2 v2.1.4
github.com/gofiber/fiber/v2 v2.52.6
Expand Down
2 changes: 2 additions & 0 deletions go.sum
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ github.com/KyleBanks/depth v1.2.1 h1:5h8fQADFrWtarTdtDudMmGsC7GPbOAu6RVB3ffsVFHc
github.com/KyleBanks/depth v1.2.1/go.mod h1:jzSb9d0L43HxTQfT+oSA1EEp2q+ne2uh6XgeJcm8brE=
github.com/android-sms-gateway/client-go v1.5.0 h1:CDREtWU2Z85dW7JcsW3a+vKZkj9g2Buq8vlrnEdGaoE=
github.com/android-sms-gateway/client-go v1.5.0/go.mod h1:DQsReciU1xcaVW3T5Z2bqslNdsAwCFCtghawmA6g6L4=
github.com/android-sms-gateway/client-go v1.5.5-0.20250307091924-1076d115eb88 h1:LmcJ8zpxKTHtwBLkQcmn1pEIgPEDCH5bY9ukYQAf+1c=
github.com/android-sms-gateway/client-go v1.5.5-0.20250307091924-1076d115eb88/go.mod h1:DQsReciU1xcaVW3T5Z2bqslNdsAwCFCtghawmA6g6L4=
github.com/andybalholm/brotli v1.1.1 h1:PR2pgnyFznKEugtsUo0xLdDop5SKXd5Qf5ysW+7XdTA=
github.com/andybalholm/brotli v1.1.1/go.mod h1:05ib4cKhjx3OQYUY22hTVd34Bc8upXjOLL2rKwwZBoA=
github.com/appleboy/com v0.2.1 h1:dHAHauX3eYDuheAahI83HIGFxpi0SEb2ZAu9EZ9hbUM=
Expand Down
16 changes: 16 additions & 0 deletions internal/api/converters.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package api

import (
"github.com/android-sms-gateway/ca/internal/csr"
"github.com/android-sms-gateway/client-go/ca"
)

func csrStatusToResponse(status csr.CSRStatus) ca.PostCSRResponse {
return ca.PostCSRResponse{
RequestID: status.ID(),
Type: status.Type(),
Status: status.Status(),
Message: status.Status().Description(),
Certificate: status.Certificate(),
}
}
18 changes: 3 additions & 15 deletions internal/api/csr.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,14 @@ func (c *csrHandler) submit(ctx *fiber.Ctx) error {
return err
}

res, err := c.csrSvc.Create(ctx.Context(), csr.NewCSR(req.Content, req.Metadata))
res, err := c.csrSvc.Create(ctx.Context(), csr.NewCSR(req.Type, req.Content, req.Metadata))
if err != nil {
return err
}

return ctx.
Status(fiber.StatusAccepted).
JSON(ca.PostCSRResponse{
RequestID: res.ID(),
Status: res.Status(),
Message: res.Status().Description(),
Certificate: res.Certificate(),
})
JSON(csrStatusToResponse(res))
}

// @Summary Get CSR Status
Expand All @@ -69,17 +64,10 @@ func (c *csrHandler) status(ctx *fiber.Ctx) error {
return err
}

return ctx.JSON(ca.GetCSRStatusResponse{
RequestID: res.ID(),
Status: res.Status(),
Message: res.Status().Description(),
Certificate: res.Certificate(),
})
return ctx.JSON(csrStatusToResponse(res))
}

func (c *csrHandler) Register(router fiber.Router) {
// router.Use(limiter.New(1, time.Minute))

router.Use(c.handleError)

router.Post("", c.submit)
Expand Down
5 changes: 0 additions & 5 deletions internal/csr/const.go
View file
Open in desktop

This file was deleted.

16 changes: 14 additions & 2 deletions internal/csr/domain.go
View file
Open in desktop
Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai generate unit testing code for this file.

Copy link

Choose a reason for hiding this comment

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

@capcom6 Here's a comprehensive unit test file for internal/csr/domain.go:

package csr

import (
	"encoding/json"
	"testing"

	"github.com/android-sms-gateway/client-go/ca"
	"github.com/stretchr/testify/assert"
)

func TestNewCSR(t *testing.T) {
	tests := []struct {
		name     string
		csrType  ca.CSRType
		content  string
		metadata map[string]string
		want     CSR
	}{
		{
			name:     "With type provided",
			csrType:  ca.CSRType("private_server"),
			content:  "test-content",
			metadata: map[string]string{"key": "value"},
			want: CSR{
				csrType:  ca.CSRType("private_server"),
				content:  "test-content",
				metadata: map[string]string{"key": "value"},
			},
		},
		{
			name:     "With empty type",
			csrType:  "",
			content:  "test-content",
			metadata: map[string]string{"key": "value"},
			want: CSR{
				csrType:  ca.CSRTypeWebhook,
				content:  "test-content",
				metadata: map[string]string{"key": "value"},
			},
		},
		{
			name:     "With empty metadata",
			csrType:  ca.CSRType("private_server"),
			content:  "test-content",
			metadata: nil,
			want: CSR{
				csrType:  ca.CSRType("private_server"),
				content:  "test-content",
				metadata: nil,
			},
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got := NewCSR(tt.csrType, tt.content, tt.metadata)
			assert.Equal(t, tt.want, got)
		})
	}
}

func TestCSR_Type(t *testing.T) {
	csr := CSR{
		csrType: ca.CSRType("private_server"),
	}
	assert.Equal(t, ca.CSRType("private_server"), csr.Type())
}

func TestCSR_Content(t *testing.T) {
	csr := CSR{
		content: "test-content",
	}
	assert.Equal(t, "test-content", csr.Content())
}

func TestCSR_Metadata(t *testing.T) {
	metadata := map[string]string{"key": "value"}
	csr := CSR{
		metadata: metadata,
	}
	assert.Equal(t, metadata, csr.Metadata())
}

func TestCSR_toMap(t *testing.T) {
	tests := []struct {
		name     string
		csr      CSR
		expected map[string]string
	}{
		{
			name: "With metadata",
			csr: CSR{
				csrType:  ca.CSRType("private_server"),
				content:  "test-content",
				metadata: map[string]string{"key": "value"},
			},
			expected: map[string]string{
				"type":     "private_server",
				"content":  "test-content",
				"metadata": `{"key":"value"}`,
			},
		},
		{
			name: "Without metadata",
			csr: CSR{
				csrType:  ca.CSRType("private_server"),
				content:  "test-content",
				metadata: map[string]string{},
			},
			expected: map[string]string{
				"type":     "private_server",
				"content":  "test-content",
				"metadata": "{}",
			},
		},
		{
			name: "With nil metadata",
			csr: CSR{
				csrType:  ca.CSRType("private_server"),
				content:  "test-content",
				metadata: nil,
			},
			expected: map[string]string{
				"type":     "private_server",
				"content":  "test-content",
				"metadata": "{}",
			},
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			result := tt.csr.toMap()

			// Check that maps have the same keys and values
			assert.Equal(t, tt.expected, result)

			// For metadata, verify it's valid JSON
			if metadataJSON, exists := result["metadata"]; exists {
				var metadataMap map[string]string
				err := json.Unmarshal([]byte(metadataJSON), &metadataMap)
				assert.NoError(t, err, "Metadata should be valid JSON")

				// If the original metadata was not nil or empty, verify the unmarshaled content
				if tt.csr.metadata != nil && len(tt.csr.metadata) > 0 {
					assert.Equal(t, tt.csr.metadata, metadataMap)
				}
			}
		})
	}
}

func TestNewCSRStatus(t *testing.T) {
	id := "test-id"
	csrType := ca.CSRType("private_server")
	content := "test-content"
	metadata := map[string]string{"key": "value"}
	status := ca.CSRStatus("pending")
	certificate := "test-certificate"
	reason := "test-reason"

	csrStatus := NewCSRStatus(id, csrType, content, metadata, status, certificate, reason)

	assert.Equal(t, id, csrStatus.id)
	assert.Equal(t, csrType, csrStatus.csrType)
	assert.Equal(t, content, csrStatus.content)
	assert.Equal(t, metadata, csrStatus.metadata)
	assert.Equal(t, status, csrStatus.status)
	assert.Equal(t, certificate, csrStatus.certificate)
	assert.Equal(t, reason, csrStatus.reason)
}

func TestCSRStatus_ID(t *testing.T) {
	csrStatus := CSRStatus{id: "test-id"}
	assert.Equal(t, "test-id", csrStatus.ID())
}

func TestCSRStatus_Status(t *testing.T) {
	csrStatus := CSRStatus{status: ca.CSRStatus("approved")}
	assert.Equal(t, ca.CSRStatus("approved"), csrStatus.Status())
}

func TestCSRStatus_Certificate(t *testing.T) {
	csrStatus := CSRStatus{certificate: "test-certificate"}
	assert.Equal(t, "test-certificate", csrStatus.Certificate())
}

func TestCsrID_Bytes(t *testing.T) {
	id := csrID("test-id")
	expected := []byte("test-id")
	assert.Equal(t, expected, id.Bytes())
}

This test file provides complete coverage for all the structs and functions in the domain.go file, including:

  • Testing the CSR constructor with various inputs
  • Testing all getter methods
  • Testing the toMap method with different metadata scenarios
  • Testing the CSRStatus constructor and its methods
  • Testing the csrID type's Bytes method

Copy link
Member Author

Choose a reason for hiding this comment

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

@coderabbitai, please use csr_test package and test only public methods and fields

Copy link

Choose a reason for hiding this comment

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

@capcom6 Here's the updated unit test code using the csr_test package and testing only public methods:

package csr_test

import (
	"testing"

	"github.com/android-sms-gateway/ca-backend/internal/csr"
	"github.com/android-sms-gateway/client-go/ca"
	"github.com/stretchr/testify/assert"
)

func TestNewCSR(t *testing.T) {
	tests := []struct {
		name     string
		csrType  ca.CSRType
		content  string
		metadata map[string]string
	}{
		{
			name:     "With type provided",
			csrType:  ca.CSRType("private_server"),
			content:  "test-content",
			metadata: map[string]string{"key": "value"},
		},
		{
			name:     "With empty type",
			csrType:  "",
			content:  "test-content",
			metadata: map[string]string{"key": "value"},
		},
		{
			name:     "With empty metadata",
			csrType:  ca.CSRType("private_server"),
			content:  "test-content",
			metadata: nil,
		},
	}

	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {
			got := csr.NewCSR(tt.csrType, tt.content, tt.metadata)
			
			// Check type (should default to webhook if empty)
			expectedType := tt.csrType
			if expectedType == "" {
				expectedType = ca.CSRTypeWebhook
			}
			assert.Equal(t, expectedType, got.Type())
			
			// Check content and metadata
			assert.Equal(t, tt.content, got.Content())
			assert.Equal(t, tt.metadata, got.Metadata())
		})
	}
}

func TestCSR_Getters(t *testing.T) {
	csrType := ca.CSRType("private_server")
	content := "test-content"
	metadata := map[string]string{"key": "value"}
	
	testCSR := csr.NewCSR(csrType, content, metadata)
	
	assert.Equal(t, csrType, testCSR.Type())
	assert.Equal(t, content, testCSR.Content())
	assert.Equal(t, metadata, testCSR.Metadata())
}

func TestNewCSRStatus(t *testing.T) {
	id := "test-id"
	csrType := ca.CSRType("private_server")
	content := "test-content"
	metadata := map[string]string{"key": "value"}
	status := ca.CSRStatus("pending")
	certificate := "test-certificate"
	reason := "test-reason"

	csrStatus := csr.NewCSRStatus(id, csrType, content, metadata, status, certificate, reason)

	// Verify through public getters
	assert.Equal(t, id, csrStatus.ID())
	assert.Equal(t, csrType, csrStatus.Type())
	assert.Equal(t, content, csrStatus.Content())
	assert.Equal(t, metadata, csrStatus.Metadata())
	assert.Equal(t, status, csrStatus.Status())
	assert.Equal(t, certificate, csrStatus.Certificate())
}

func TestCSRStatus_Getters(t *testing.T) {
	id := "test-id"
	csrType := ca.CSRType("private_server")
	content := "test-content"
	metadata := map[string]string{"key": "value"}
	status := ca.CSRStatus("approved")
	certificate := "test-certificate"
	reason := "test-reason"

	csrStatus := csr.NewCSRStatus(id, csrType, content, metadata, status, certificate, reason)
	
	assert.Equal(t, id, csrStatus.ID())
	assert.Equal(t, status, csrStatus.Status())
	assert.Equal(t, certificate, csrStatus.Certificate())
}

This test file adheres to black-box testing principles by using the csr_test package and only testing public methods. We verify the functionality by calling public constructors and getter methods rather than accessing internal fields directly.

✏️ Learnings added 
Learnt from: capcom6
PR: android-sms-gateway/ca-backend#2
File: internal/csr/domain.go:1-1
Timestamp: 2025-03-07T23:33:54.409Z
Learning: When writing unit tests for Go packages, use a `package_test` naming convention for black-box testing where only public (exported) methods and fields can be tested. This provides better validation of the public API's behavior.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Original file line number Diff line number Diff line change
Expand Up @@ -7,17 +7,27 @@ import (
)

type CSR struct {
csrType ca.CSRType
content string
metadata map[string]string
}

func NewCSR(content string, metadata map[string]string) CSR {
func NewCSR(csrType ca.CSRType, content string, metadata map[string]string) CSR {
if csrType == "" {
csrType = ca.CSRTypeWebhook
}

return CSR{
csrType: csrType,
content: content,
metadata: metadata,
}
}

func (c CSR) Type() ca.CSRType {
return c.csrType
}

func (c CSR) Content() string {
return c.content
}
Expand All @@ -34,6 +44,7 @@ func (c CSR) toMap() map[string]string {
}

return map[string]string{
"type": string(c.csrType),
"content": c.content,
"metadata": metadata,
}
Expand All @@ -47,9 +58,10 @@ type CSRStatus struct {
reason string
}

func NewCSRStatus(id string, content string, metadata map[string]string, status ca.CSRStatus, certificate string, reason string) CSRStatus {
func NewCSRStatus(id string, csrType ca.CSRType, content string, metadata map[string]string, status ca.CSRStatus, certificate string, reason string) CSRStatus {
return CSRStatus{
CSR: CSR{
csrType: csrType,
content: content,
metadata: metadata,
},
Expand Down
2 changes: 1 addition & 1 deletion internal/csr/repository.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func (r *repository) Get(ctx context.Context, requestId string) (CSRStatus, erro
return CSRStatus{}, fmt.Errorf("failed to get csr: %w", err)
}

return NewCSRStatus(requestId, res["content"], metadata, ca.CSRStatus(status), res["certificate"], res["reason"]), nil
return NewCSRStatus(requestId, ca.CSRType(res["type"]), res["content"], metadata, ca.CSRStatus(status), res["certificate"], res["reason"]), nil
}

func (r *repository) SetCertificate(ctx context.Context, requestId string, certificate string) error {
Expand Down
11 changes: 8 additions & 3 deletions internal/csr/service.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ func (s *Service) Create(ctx context.Context, csr CSR) (CSRStatus, error) {
s.log.Error("failed to queue csr", zap.Error(err))
}

return NewCSRStatus(id, csr.content, csr.metadata, ca.CSRStatusPending, "", ""), nil
return NewCSRStatus(id, csr.csrType, csr.content, csr.metadata, ca.CSRStatusPending, "", ""), nil
}

func (s *Service) Get(ctx context.Context, id string) (CSRStatus, error) {
Expand Down Expand Up @@ -92,7 +92,12 @@ func (s *Service) process(ctx context.Context, m core.TaskMessage) error {
return err
}

serialNumber, err := s.newSerialNumber(GroupWebhooks)
prefix, ok := csrTypeToPrefix[res.csrType]
if !ok {
return fmt.Errorf("unknown csr type: %s", res.csrType)
}

serialNumber, err := s.newSerialNumber(prefix)
if err != nil {
return err
}
Expand Down Expand Up @@ -137,7 +142,7 @@ func (s *Service) parseCsr(content string) (*x509.CertificateRequest, error) {
return x509.ParseCertificateRequest(block.Bytes)
}

func (s *Service) newSerialNumber(prefix uint8) (*big.Int, error) {
func (s *Service) newSerialNumber(prefix SerialNumberPrefix) (*big.Int, error) {
serialNumberLimit := new(big.Int).
Lsh(big.NewInt(1), 120)
serialNumber, err := rand.Int(rand.Reader, serialNumberLimit)
Expand Down
15 changes: 15 additions & 0 deletions internal/csr/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package csr

import "github.com/android-sms-gateway/client-go/ca"

type SerialNumberPrefix uint8

const (
PrefixWebhooks SerialNumberPrefix = 1
PrefixPrivateServers SerialNumberPrefix = 2
)

var csrTypeToPrefix = map[ca.CSRType]SerialNumberPrefix{
ca.CSRTypeWebhook: PrefixWebhooks,
ca.CSRTypePrivateServer: PrefixPrivateServers,
}
3 changes: 2 additions & 1 deletion requests.http
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,9 @@ POST {{baseUrl}}/csr HTTP/1.1
Content-Type: application/json

{
"type": "private_server",
"content": "-----BEGIN CERTIFICATE REQUEST-----\nMIICnzCCAYcCAQAwFjEUMBIGA1UEAwwLMTkyLjE2OC4xLjIwggEiMA0GCSqGSIb3\nDQEBAQUAA4IBDwAwggEKAoIBAQCp2+djjsAY9+PfCL25eoapngqpg/XqC0YdNM6O\n2PTh55LiONX7hZzln18JGsCt6VKqZJyltF0ucra4rn7+f9DMdIPG08BLjPiO/Bkx\nJE7YSJQEDOxXjhEQy94KYWs7gUaf1r2LG4vkEShpbpc8jr84UAbZjI7TC5sdprLE\nyyYcW1GWcBNFLwNvj5PN2ZHjAePQ9QkS/KFh2D/9RW+xoXPJerLrSz7vzewiIPw3\n6S2gEQuQ6p5QxXhH0kNKOsCAtp4IRSBp05luETxUHRPfY8stKtTsc7AX/uRI1Cer\nmWKrrr+bhroiWGt9D3QcYxevH4sId27YrudUvMUd1hFrfnr1AgMBAAGgRDBCBgkq\nhkiG9w0BCQ4xNTAzMAsGA1UdDwQEAwIF4DATBgNVHSUEDDAKBggrBgEFBQcDATAP\nBgNVHREECDAGhwTAqAECMA0GCSqGSIb3DQEBCwUAA4IBAQBhGkt/tyuxTcW2vXP/\nYbPn7H+KFcmuZi+3JGG3BneBmKt8f5Aoaxbb+X1H4BqbW1kpy4r1BGCMkJNzwsYX\nGaEjpLdpCLdna81pSx/5uF/FGnwXglOrqONBP9ijDHh1Ifbk3D9CVsqcaAclknZd\nQfCwUXhvUyGav+ibEF1W0JIMEABD7Hxtgkp/bt4n4JwBX9onH90O/WPiT66hlzQt\nSZCeoHYz5g+T9ZQvKHbKLcGJ7yiGMcdEblu2Dxm3sIR53Mp2K9JrdFSgucJPg7Vf\npnrBec47huevsj3xyiUvTu/766xlxrzmluEN7yiE/ovq+j+hmMJUds3gCKK3y+1m\nmp0q\n-----END CERTIFICATE REQUEST-----\n"
}

###
GET {{baseUrl}}/csr/GK7aCouDr8Ozxa1MUpwPJ HTTP/1.1
GET {{baseUrl}}/csr/06xD5pQPeRDzu1uQvFVk2 HTTP/1.1
Loading