From d182c9d52e7e27bb3d5a698d4788a3f0919012bc Mon Sep 17 00:00:00 2001 From: jwijenbergh Date: Wed, 13 Mar 2024 16:22:32 +0100 Subject: Client + Server: Refactor adding a server by first adding then auth Otherwise we go to the main state after auth is done and the server is not yet added --- internal/server/custom.go | 32 +++++++++++++++----------------- internal/server/institute.go | 31 ++++++++++++++++--------------- internal/server/secureinternet.go | 33 +++++++++++++++++---------------- 3 files changed, 48 insertions(+), 48 deletions(-) (limited to 'internal') diff --git a/internal/server/custom.go b/internal/server/custom.go index 10e9a28..d2679cf 100644 --- a/internal/server/custom.go +++ b/internal/server/custom.go @@ -2,7 +2,6 @@ package server import ( "context" - "time" "github.com/eduvpn/eduvpn-common/internal/api" "github.com/eduvpn/eduvpn-common/internal/config/v2" @@ -14,7 +13,7 @@ import ( // `ctx` is the context used for cancellation // `id` is the identifier of the server, the base URL // `na` specifies whether or not we want to add the server without doing authorization now -func (s *Servers) AddCustom(ctx context.Context, id string, na bool) (*Server, error) { +func (s *Servers) AddCustom(ctx context.Context, id string, na bool) error { sd := api.ServerData{ ID: id, Type: server.TypeCustom, @@ -22,25 +21,24 @@ func (s *Servers) AddCustom(ctx context.Context, id string, na bool) (*Server, e BaseAuthWK: id, } - var a *api.API - var err error - if !na { - // Authorize by creating the API object - a, err = api.NewAPI(ctx, s.clientID, sd, s.cb, nil) - if err != nil { - return nil, err - } - } - - err = s.config.AddServer(id, server.TypeCustom, v2.Server{LastAuthorizeTime: time.Now()}) + err := s.config.AddServer(id, server.TypeCustom, v2.Server{}) if err != nil { - return nil, err + return err } - cust := s.NewServer(id, server.TypeCustom, a) - // Return the server with the API + // no authorization should be triggered, return + if na { + return nil + } - return &cust, nil + // Authorize by creating the API object + _, err = api.NewAPI(ctx, s.clientID, sd, s.cb, nil) + if err != nil { + // authorization has failed, remove the server again + s.config.RemoveServer(id, server.TypeCustom) + return err + } + return nil } // GetCustom gets a custom server diff --git a/internal/server/institute.go b/internal/server/institute.go index aa032c7..195c2ef 100644 --- a/internal/server/institute.go +++ b/internal/server/institute.go @@ -2,7 +2,6 @@ package server import ( "context" - "time" "github.com/eduvpn/eduvpn-common/internal/api" "github.com/eduvpn/eduvpn-common/internal/config/v2" @@ -16,11 +15,11 @@ import ( // `disco` are the discovery servers // `id` is the identifier for the server, the base url // `na` is true when authorization should not be triggered -func (s *Servers) AddInstitute(ctx context.Context, disco *discovery.Discovery, id string, na bool) (*Server, error) { +func (s *Servers) AddInstitute(ctx context.Context, disco *discovery.Discovery, id string, na bool) error { // This is basically done to double check if the server is part of the institute access section of disco dsrv, err := disco.ServerByURL(id, "institute_access") if err != nil { - return nil, err + return err } sd := api.ServerData{ @@ -30,22 +29,24 @@ func (s *Servers) AddInstitute(ctx context.Context, disco *discovery.Discovery, BaseAuthWK: dsrv.BaseURL, } - var a *api.API - if !na { - // Authorize by creating the API object - a, err = api.NewAPI(ctx, s.clientID, sd, s.cb, nil) - if err != nil { - return nil, err - } + err = s.config.AddServer(dsrv.BaseURL, server.TypeInstituteAccess, v2.Server{}) + if err != nil { + return err } - err = s.config.AddServer(dsrv.BaseURL, server.TypeInstituteAccess, v2.Server{LastAuthorizeTime: time.Now()}) - if err != nil { - return nil, err + // no authorization should be triggered, return + if na { + return nil } - inst := s.NewServer(dsrv.BaseURL, server.TypeInstituteAccess, a) - return &inst, nil + // Authorize by creating the API object + _, err = api.NewAPI(ctx, s.clientID, sd, s.cb, nil) + if err != nil { + // authorization has failed, remove the server again + s.config.RemoveServer(dsrv.BaseURL, server.TypeInstituteAccess) + return err + } + return nil } // GetInstitute gets an institute access server diff --git a/internal/server/secureinternet.go b/internal/server/secureinternet.go index 0d06a55..3377650 100644 --- a/internal/server/secureinternet.go +++ b/internal/server/secureinternet.go @@ -3,7 +3,6 @@ package server import ( "context" "errors" - "time" "github.com/eduvpn/eduvpn-common/internal/api" "github.com/eduvpn/eduvpn-common/internal/config/v2" @@ -18,9 +17,9 @@ import ( // `disco` are the discovery servers // `orgID` is the organiztaion ID // `na` specifies whether or not authorization should be triggered when adding -func (s *Servers) AddSecure(ctx context.Context, disco *discovery.Discovery, orgID string, na bool) (*Server, error) { +func (s *Servers) AddSecure(ctx context.Context, disco *discovery.Discovery, orgID string, na bool) error { if s.config.HasSecureInternet() { - return nil, errors.New("a secure internet server already exists") + return errors.New("a secure internet server already exists") } dorg, dsrv, err := disco.SecureHomeArgs(orgID) if err != nil { @@ -28,7 +27,7 @@ func (s *Servers) AddSecure(ctx context.Context, disco *discovery.Discovery, org // Note that in the docs it states that it only should happen when the Org ID doesn't exist // However, this is nice as well because it also catches the error where the SecureInternetHome server is not found disco.MarkOrganizationsExpired() - return nil, err + return err } sd := api.ServerData{ @@ -41,22 +40,24 @@ func (s *Servers) AddSecure(ctx context.Context, disco *discovery.Discovery, org }, } - var a *api.API - if !na { - // Authorize by creating the API object - a, err = api.NewAPI(ctx, s.clientID, sd, s.cb, nil) - if err != nil { - return nil, err - } + err = s.config.AddServer(orgID, server.TypeSecureInternet, v2.Server{CountryCode: dsrv.CountryCode}) + if err != nil { + return err } - err = s.config.AddServer(orgID, server.TypeSecureInternet, v2.Server{CountryCode: dsrv.CountryCode, LastAuthorizeTime: time.Now()}) - if err != nil { - return nil, err + // no authorization should be triggered, return + if na { + return nil } - sec := s.NewServer(orgID, server.TypeSecureInternet, a) - return &sec, nil + // Authorize by creating the API object + _, err = api.NewAPI(ctx, s.clientID, sd, s.cb, nil) + if err != nil { + // authorization has failed, remove the server again + s.config.RemoveServer(orgID, server.TypeSecureInternet) + return err + } + return nil } // GetSecure gets a secure internet server -- cgit v1.2.3