From bc02da786d9a4a02bd06b3dd4fa99a82b54980d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Budai?= <obudai@redhat.com>
Date: Wed, 19 Aug 2020 14:48:26 +0200
Subject: [PATCH] upload/koji: ensure that Koji type instance is always
 logged-in

Previously, Koji instance could be both logged-in and not logged-in.
This change disallows it: Now, the Koji instance is created by calling
koji.Login, so it must be always logged-in. This change should lead to more
robust code.
---
 cmd/osbuild-koji/main.go          |  9 +----
 internal/upload/koji/koji.go      | 63 ++++++++++++++++---------------
 internal/upload/koji/koji_test.go |  5 +--
 3 files changed, 36 insertions(+), 41 deletions(-)

diff --git a/cmd/osbuild-koji/main.go b/cmd/osbuild-koji/main.go
index f26994eea..08706e0e8 100644
--- a/cmd/osbuild-koji/main.go
+++ b/cmd/osbuild-koji/main.go
@@ -4,6 +4,7 @@ import (
 	"flag"
 	"fmt"
 	"log"
+	"net/http"
 	"os"
 	"path"
 	"time"
@@ -38,13 +39,7 @@ func main() {
 	}
 	defer file.Close()
 
-	k, err := koji.New(server)
-	if err != nil {
-		println(err.Error())
-		return
-	}
-
-	err = k.Login("osbuild", "osbuildpass")
+	k, err := koji.Login(server, "osbuild", "osbuildpass", http.DefaultTransport)
 	if err != nil {
 		println(err.Error())
 		return
diff --git a/internal/upload/koji/koji.go b/internal/upload/koji/koji.go
index a0fea3fa6..7e5a833ce 100644
--- a/internal/upload/koji/koji.go
+++ b/internal/upload/koji/koji.go
@@ -106,15 +106,42 @@ type CGImportResult struct {
 	BuildID int `xmlrpc:"build_id"`
 }
 
-func New(server string) (*Koji, error) {
-	k := &Koji{}
-	client, err := xmlrpc.NewClient(server, http.DefaultTransport)
+func Login(server, user, password string) (*Koji, error) {
+	// Create a temporary xmlrpc client with the default http transport
+	// as we don't need sessionID, key nor callnum yet.
+	loginClient, err := xmlrpc.NewClient(server, http.DefaultTransport)
 	if err != nil {
 		return nil, err
 	}
-	k.xmlrpc = client
-	k.server = server
-	return k, nil
+
+	args := []interface{}{user, password}
+	var reply struct {
+		SessionID  int64  `xmlrpc:"session-id"`
+		SessionKey string `xmlrpc:"session-key"`
+	}
+	err = loginClient.Call("login", args, &reply)
+	if err != nil {
+		return nil, err
+	}
+
+	// Create the final xmlrpc client with our custom RoundTripper handling
+	// sessionID, sessionKey and callnum
+	kojiTransport := &Transport{
+		sessionID:  reply.SessionID,
+		sessionKey: reply.SessionKey,
+		callnum:    0,
+	}
+
+	client, err := xmlrpc.NewClient(server, kojiTransport)
+	if err != nil {
+		return nil, err
+	}
+
+	return &Koji{
+		xmlrpc:    client,
+		server:    server,
+		transport: kojiTransport,
+	}, nil
 }
 
 // GetAPIVersion gets the version of the API of the remote Koji instance
@@ -128,26 +155,6 @@ func (k *Koji) GetAPIVersion() (int, error) {
 	return version, nil
 }
 
-// Login sets up a new session with the given user/password
-func (k *Koji) Login(user, password string) error {
-	args := []interface{}{user, password}
-	var reply struct {
-		SessionID  int64  `xmlrpc:"session-id"`
-		SessionKey string `xmlrpc:"session-key"`
-	}
-	err := k.xmlrpc.Call("login", args, &reply)
-	if err != nil {
-		return err
-	}
-
-	k.transport = &Transport{
-		sessionID:  reply.SessionID,
-		sessionKey: reply.SessionKey,
-		callnum:    0,
-	}
-	return nil
-}
-
 // Logout ends the session
 func (k *Koji) Logout() error {
 	err := k.xmlrpc.Call("logout", nil, nil)
@@ -279,10 +286,6 @@ type Transport struct {
 // us to adjust the URL per-call (these arguments should really be in
 // the body).
 func (rt *Transport) RoundTrip(req *http.Request) (*http.Response, error) {
-	if rt.sessionKey == "" {
-		return http.DefaultTransport.RoundTrip(req)
-	}
-
 	// Clone the request, so as not to alter the passed in value.
 	rClone := new(http.Request)
 	*rClone = *req
diff --git a/internal/upload/koji/koji_test.go b/internal/upload/koji/koji_test.go
index 0cd8bb543..a5f2a033c 100644
--- a/internal/upload/koji/koji_test.go
+++ b/internal/upload/koji/koji_test.go
@@ -31,10 +31,7 @@ func TestKojiImport(t *testing.T) {
 	// koji needs to specify a directory to which the upload should happen, let's reuse the build name
 	uploadDirectory := buildName
 
-	// authenticate
-	k, err := koji.New(server)
-	require.NoError(t, err)
-	err = k.Login(user, password)
+	k, err := koji.Login(server, user, password)
 	require.NoError(t, err)
 
 	defer func() {